<< Back to previous view

[CLJ-1910] Namespaced maps Created: 07/Apr/16  Updated: 21/Jan/18  Resolved: 23/Jun/16

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: Alex Miller
Resolution: Completed Votes: 1
Labels: print, reader

Attachments: Text File clj-1910-2.patch     Text File clj-1910.patch    
Patch: Code and Test
Approval: Ok

 Description   

A common usage of namespaced keywords and symbols is in providing attribute disambiguation in map contexts:

{:person/first "Han" :person/last "Solo" :person/ship 
  {:ship/name "Millenium Falcon" :ship/model "YT-1300f light freighter"}}

The namespaces provide value (disambiguation) but have the downside of being repetitive and verbose.

Namespaced maps are a reader (and printer) feature to specify a namespace context for a map.

  • Namespaced maps combine a default namespace with a map and yield a map.
  • Namespaced maps are reader macros starting with #: or #::, followed by a normal map definition.
    • #:sym indicates that sym is the default namespace for the map to follow.
    • #:: indicates that the default namespace auto-resolves to the current namespace.
    • #::sym indicates that sym should be auto-resolved using the current namespace's aliases OR any fully-qualified loaded namespace.
      • These rules match the rules for auto-resolved keywords.
  • A namespaced map is read with the following differences from normal maps:
    • A keyword or symbol key without a namespace is read with the default namespace as its namespace.
    • Keys that are not symbols or keywords are not affected.
    • Keys that specify an explicit namespace are not affected EXCEPT the special namespace _, which is read with NO namespace. This allows the specification of bare keys in a namespaced map.
    • Values are not affected.
    • Nested map keys are not affected.
  • The edn reader supports #: but not #:: with the same rules as above.
  • Maps will be printed in namespaced map form only when:
    • All map keys are keywords or symbols
    • All map keys are namespaced
    • All map keys have the same namespace

Examples:

;; same as above - notice you can nest #: maps and this is a case where the printer roundtrips
user=> #:person{:first "Han" :last "Solo" :ship #:ship{:name "Millenium Falcon" :model "YT-1300f light freighter"}}
#:person{:first "Han" :last "Solo" :ship #:ship{:name "Millenium Falcon" :model "YT-1300f light freighter"}}

;; effects on keywords with ns, without ns, with _ ns, and non-kw
user=> #:foo{:kw 1, :n/kw 2, :_/bare 3, 0 4}
{:foo/kw 1, :n/kw 2, :bare 3, 0 4}

;; auto-resolved namespaces (will use user as the ns)
user=> #::{:kw 1, :n/kw 2, :_/bare 3, 0 4}
:user/kw 1, :n/kw 2, :bare 3, 0 4}

;; auto-resolve alias s to clojure.string
user=> (require '[clojure.string :as s])
nil
user=> #::s{:kw 1, :n/kw 2, :_/bare 3, 0 4}
{:clojure.string/kw 1, :n/kw 2, :bare 3, 0 4}

;; to show symbol changes, we'll quote the whole thing to avoid evaluation
user=> '#::{a 1, n/b 2, _/c 3}
{user/a 1, n/b 2, c 3}

;; edn reader also supports (only) the #: syntax
user=> (clojure.edn/read-string "#:person{:first \"Han\" :last \"Solo\" :ship #:ship{:name \"Millenium Falcon\" :model \"YT-1300f light freighter\"}}")
#:person{:first "Han", :last "Solo", :ship #:ship{:name "Millenium Falcon", :model "YT-1300f light freighter"}}

Patch: clj-1910-2.patch

Screener notes:

  • Autoresolution supports fully-qualified loaded namespaces (like auto-resolved keywords)
  • TODO: pprint support for namespaced maps
  • TODO: printer flag to suppress printing namespaced maps


 Comments   
Comment by Nicola Mometto [ 08/Apr/16 3:57 AM ]

1- yes please. that's consistent with how tagged literals work.
2- no please. that would make the proposed syntax useless for e.g. Datomic schemas, for which I think this would be a good fit to reduce noise
3- yes please
4- yes please, consistency over print methods is important

Comment by Nicola Mometto [ 08/Apr/16 4:00 AM ]

Quoting from a post I wrote on the clojure-dev ML:

  • I really don't like the idea of special-casing `_` here, users are already confused about idioms like `[.. & _]` thinking that `_` is some special token/variable. Making it an actual special token in some occasion wouldn't help.
  • I also don't like how we're making the single `:` auto-qualify keywords when used within the context of a qualifying-map. Auto-qualifying keywords has always been the job of the double `::`, changing this would introduce (IMO) needless cognitive overhead.
  • The current impl treats `#:foo{'bar 1}` and `'#:foo{bar 1}` differently. I can see why is that, but the difference might be highly unintuitive to some.
  • The current proposal makes it feel like quote is auto-qualifying symbols , when that has always been the job of syntax-quote. I know that's not correct, but that's how it's perceived.

Here's an alternative syntax proposal that handles all those issues:

  • No #::, only #:foo or #::foo
  • No auto-resolution of symbols when the namespaced-map is quoted, only when syntax-quoted
  • No special-casing of `_`
  • No auto-resolution of single-colon keywords

Here's how the examples in the ticket description would look:

#:person{::first "Han", ::last "Solo", ::ship #:ship{::name "Millenium Falcon", ::model "YT-1300f light freighter"}}
;=> {:person/first "Han" :person/last "Solo" :person/ship {:ship/name "Millenium Falcon" :ship/model "YT-1300f light freighter"}}

#:foo{::kw 1, :n/kw 2, :bare 3, 0 4}
;=> {:foo/kw 1, :n/kw 2, :bare 3, 0 4}

{::kw 1, :n/kw 2, bare 3, 0 4}
;=> {:user/kw 1, :n/kw 2, :bare 3, 0 4}

Note in the previous example how we don't need `#::` at all – `::` already does that job for us

(require '[clojure.string :as s])
#::s{::kw 1, :n/kw 2, bare 3, 0 4}
;=> {:clojure.string/kw 1, :n/kw 2, :bare 3, 0 4}

`{a 1, n/b 2, ~'c 3}
;=> {user/a 1, n/b 2, c 3}

Again, no need for `#::` here, we can just rely on the existing auto-qualifying behaviour of `.

`#:foo{a 1, n/b 2}
;=> {foo/a 1, n/b 2}

I think this would be more consistent with the existing behaviour – it's basically just making `#:foo` or `#::foo` mean: in the top-level keys of the following map expression, resolve keywords/symbols as if ns was bound to `foo`, rather than introducing new resolution rules and special tokens.

I realize that this proposal wouldn't work with EDNReader as-is, given its lack of support for `::` and "`". I don't have a solution to that other than "let's just bite the bullet and implement them there too", but maybe that's not acceptable.

Comment by Alex Miller [ 08/Apr/16 8:45 AM ]

Nicola, thanks for the proposal, we talked through it. We share your dislike for :_/kw syntax and you should consider that a placeholder for this behavior for the moment - it may be removed or replaced before we get to a published release.

For the rest of it:

  • requiring syntax quote is a non-starter
  • supporting a mixture of default ns and the current ns is important and this is not possible with your proposal. Like #:foo{:bar 1 ::baz 2}.
  • there is a lot of value to changing the scope of a map without modifying the contents, which is an advantage of the syntax in the ticket
Comment by Christophe Grand [ 08/Apr/16 10:31 AM ]

Why restrict this feature to a single namespace? (this doesn't preclude a shorthand for the single mapping) I'd like to locally define aliases (and default ns).

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

We already have namespace level aliases. You can use :: in the map to leverage those aliases (independently from the default ns):

(ns app 
  (:require [my.domain :as d]
            [your.domain :as y]))

#::{:svc 1, ::d/name 2, ::y/name 3}

;;=> {:app/svc 1, :my.domain/name 2, :your.domain/y 3}
Comment by Christophe Grand [ 11/Apr/16 4:03 AM ]

Alex, if existing namespace level aliases are enough when there's more than one namespace used in the key set I fail to understand the real value of this proposal.

Okay I'm lying a little: there are no aliases in edn, so this would bring aliases to edn (and allows printers to factor/alias namespaces out). And for Clojure code you can't define an alias to a non-existing namespace – and I believe that this implementation wouldn't check namespace existence when resolving the default ns #:person{:name}.

Still my points hold for edn (and that's where the value of this proposal seems to be): why not allows local aliases too?

#:person #:employee/e {:name "John Smith", :e/eid "012345"}
;=> {:person/name "John Smith", :employee/eid "012345"}

I have another couple of questions:

  • should it apply to other datatypes?
  • should it be transitive?
Comment by Alex Miller [ 28/Apr/16 1:33 PM ]

New patch rev supports spaces between the namespace part #:foo and the map in both LispReader and EdnReader.

Comment by Tim Gilbert [ 23/Sep/16 1:12 AM ]

Any news on the flag to prevent this behavior (mentioned in the screener notes)?

I ask because (pr-str) in Clojure 1.9.0-alpha12 currently produces EDN that can't be consumed by (cljs.reader/read-string) in ClojureScript 1.9.229 (the ClojureScript consumption side is tracked in CLJS-1706).

It would be nice to have a way to disable this behavior until ClojureScript reaches parity, and a lot of fairly widely-used open-source libraries use plain (pr-str) to generate EDN output - for example, ring-middleware-format.

Comment by Alex Miller [ 23/Sep/16 8:37 AM ]

Yes, that was implemented in a separate ticket (CLJ-1993) in Clojure 1.9.0-alpha11. There is now a print-namespace-maps dynvar. It defaults to false, but is set to true in the standard REPL.

So at the REPL you can (set! print-namespace-maps false) to turn it off.

If you're doing stuff in your program outside a REPL, it will be off by default so you probably don't need to do anything as of alpha11.

Comment by Alex Miller [ 23/Sep/16 8:38 AM ]

that's *print-namespace-maps* above, sorry for the formatting

Comment by Vitalie Spinu [ 14/Aug/17 5:37 AM ]

Any plans to extend this to vectors? Would be useful in a number of contexts:

(s/keys :req #:foo[:a :b])
(select-keys m #:foo[:a :b])
(get-in m #:foo[:a :b])
Comment by Alex Miller [ 14/Aug/17 7:01 AM ]

We've considered extending it to vectors and that might be a future addition. One thing with that is it drives the question of whether the namespace syntax should be applicable to all collections and the syntax clashes with the set literal syntax is pretty ugly: #:foo#{:a :b}.

Comment by Vitalie Spinu [ 26/Aug/17 11:00 AM ]

How do I bind *print-namespace-maps* in REPL?

with-bindings hardcodes it to true. Looks like an ugly exception, as all the other bindings pick the root value there.

Comment by Alex Miller [ 26/Aug/17 3:04 PM ]

If you're in a clojure.main repl, as you note it's already bound, so you can just use `set!`:

user=> (set! *print-namespace-maps* false)
false
user=> {:a/b 1}
{:a/b 1}

This won't work in nrepl-based repls that don't set it though, so you can also use `binding`:

user=> (binding [*print-namespace-maps* false] (println {:a/b 1}))
{:a/b 1}
Comment by Vitalie Spinu [ 27/Aug/17 7:51 AM ]

Thanks for the clarification. It's NREPL issue then. I can happily set! other globals there but not this one.

Comment by Chenyu Li [ 19/Jan/18 6:12 PM ]

Is namespace that starts with number not allowed in namespaced maps reader macros in clojure 1.9?

user=> {:3/a 1}
#:3{:a 1}

user=> #:3{:a 1}
RuntimeException Namespaced map must specify a valid namespace: 3 clojure.lang.Util.runtimeException (Util.java:221)

I'm using clojure 1.9.0.

Although the same reader macro #:3{:a 1} is accepted in clojurescript/1.9.946

Comment by Andy Fingerhut [ 21/Jan/18 4:54 AM ]

Clearly from your example where Clojure 1.9.0 throws an exception when attempting to read the input #:3{:a 1}, it does not accept a namespace that starts with a number.

Perhaps your question is: "Is this a bug in Clojure 1.9.0, or intended behavior?"

Comment by Andy Fingerhut [ 21/Jan/18 4:56 AM ]

Chenyu Li, you may want to consider creating a new JIRA issue with that question, since this ticket is already closed as resolved, and such a question in a comment here is more likely to go unnoticed.





[CLJ-2308] Reduce based some Created: 04/Jan/18  Updated: 20/Jan/18

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

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

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

 Description   

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

To get this to work I had to:

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

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

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

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





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

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

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

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

 Description   

Before patch:

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

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

After patch:

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

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



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

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

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

Set fix version





[CLJ-2246] spec.test/check returns the wrong value of :failure for failing tests Created: 02/Oct/17  Updated: 18/Jan/18

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

Type: Defect Priority: Major
Reporter: Khalid Jebbari Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

[org.clojure/clojure "1.9.0-beta1"]
[org.clojure/spec.alpha "0.1.123"]
[org.clojure/test.check "0.10.0-alpha2" :scope "test"]


Attachments: Text File 0001-Use-result-type-to-test-for-failure-in-abbrev-result.patch    
Approval: Triaged

 Description   

When trying to manually wire clojure.test and spec for fdef'ed functions, I realized that spec.test/check returns a map with [:failure false] for failing tests. I'm (almost) sure it should return true, because spec.test/abbrev-result receives as argument the return of spec.test/check and tests the value of :failure. I couldn't produce a case where spec.test/check returned [:failure true] for failing tests. For information it returns a map without the key :failure for passing test.

Here's a simple reproduction case in the REPL :

(defn foo-fn [x] "bar")

(s/fdef foo-fn
:args (s/cat :x any?)
:ret string?)

(stest/check `foo-fn) ;; => no error, normal small map printed, no :failure entry

(defn foo-fn [x] 1)

(stest/check `foo-fn) ;; => full error map, with :failure equals false. :failure shown below

(-> (stest/check `foo-fn) first :failure) ;; => false



 Comments   
Comment by Gary Fredericks [ 02/Oct/17 7:34 PM ]

This is not a bug, though it is confusing.

stest/check is passing through (at least part of) the return value from clojure.test.check/quickcheck, which returns the value returned by the property, which is evaluated for thruthiness to determine pass vs fail. Since a non-truthy value can only be false or nil, you don't learn very much by looking at it (though it's possible that the :failure key could also have an exception under it, which would be more informative).

The next release of test.check should have a more useful and less confusing way of extracting information from a test, but I don't know for sure how that would look when using stest/check.

Comment by Khalid Jebbari [ 03/Oct/17 3:43 AM ]

I still don't understand why you don't consider it a bug. The docstring of `stest/check` says ":failure optional test failure". I expect to have a `:failure` key only when the tests fail (hence the wording optional), with some valuable info.

I'm using the `stest/abbrev-result` to display the output of `stest/check`, which expects `:failure` to be truthy to display all the valuable failure information. If `stest/check` returns `false`, there's a mismatch I consider a bug. The docstring of `stest/abbrev-result` explicitly says it receives as argument the result of check, so I'm not forcing a square peg into a round hole.

Thank you for answering me so fast and for your time.

Comment by Michael Glaesemann [ 15/Jan/18 6:23 PM ]

The false value for :failure is definitely confusing. The stest/abbrev-result is very confounding in the {:failure false} as it doesn't provide additional information that failures usual do,as Khalid Jebbari points out.

(defn abbrev-result
  "Given a check result, returns an abbreviated version
suitable for summary use."
  [x]
  (if (:failure x)
    (-> (dissoc x ::stc/ret)
        (update :spec s/describe)
        (update :failure unwrap-failure))
    (dissoc x :spec ::stc/ret)))

Here's an example showing how misleading it can be:

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

(alias 'stc 'clojure.spec.test.check)

(defn adder [a b]
  (+ a b))

(s/fdef adder
        :args (s/cat :a int? :b int?)
        :ret string?)

(-> (stest/check `adder) first stest/abbrev-result)
;; => {:sym ex.check-test/adder, :failure false}

;; Writing an alternative version of `abbrev-result` that
;; checks for `true`

(defn- failure-type [x] (::s/failure (ex-data x)))
(defn- unwrap-failure [x] (if (failure-type x) (ex-data x) x))

(defn- abbrev-result [x]
  (let [failure (:failure x)]
    (if-not (or (true? failure)
                (nil? failure))
      (-> (dissoc x ::stc/ret)
          (update :spec s/describe)
          (update :failure unwrap-failure))
      (dissoc x :spec ::stc/ret))))

(-> (stest/check `adder) first abbrev-result)
;; => {:spec (fspec :args (cat :a int? :b int?) :ret string? :fn nil),
;;     :sym ex.check-test/adder,
;;     :failure false}

Again, note that any truthy :failure value is going to provide the additional details, while falsey :failure values will not.

I understand the motivation for not changing the value of the :failure key. If that value is going to be maintained, I think stest/abbrev-result should be updated to likewise test explicitly for nil and true, rather than truthy for consistency with the result of stest/check.

Comment by Khalid Jebbari [ 16/Jan/18 8:13 AM ]

Thanks a lot Michael. In the end I went with a small variation, where I don't `(dissoc x ::stc/ret)` but keep the whole check result because it includes the stack trace, the spec/explain-data map, the shrunk failing case etc.

Comment by Michael Glaesemann [ 17/Jan/18 9:19 PM ]

Looks like abbrev-result should use result-type to test whether the check passed. Attaching a patch which does this. If you'd like tests to accompany it, I'm happy to resubmit.

Edit: Nope: that was too hasty. Will investigate and resubmit. Sorry for the noise.

Edit 2: Second guessing myself incorrectly. I'm pretty sure I was correct the first time. I think the patch is good.

Comment by Gary Fredericks [ 18/Jan/18 6:23 AM ]

note that there are unreleased changes on the master branch of test.check that may influence the best thing to do here. this stuff in particular.

Comment by Michael Glaesemann [ 18/Jan/18 3:40 PM ]

Thanks, Gary Fredericks. The Result protocol is something to keep in mind. I think using result-type is the right abstraction at the level of abbrev-result. I would think Result/passing? would be used when decorating the quick-check results in make-check-result in lieu of the (true? result) call. Does that make sense to you?





[CLJ-2312] Avoid using keywords as sentinel values in transducers Created: 12/Jan/18  Updated: 12/Jan/18

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

Type: Defect Priority: Minor
Reporter: Rick Moynihan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: security, transducers
Environment:

All environments



 Description   

The use of keywords as sentinels in transducers could in rare circumstances expose some applications to bugs and potential security risks.

(sequence (partition-by keyword) ["1" "none" "2" "clojure.core/none" "3" "4"])
;(["1"] ["none"] ["2"] ["clojure.core/none" "3"] ["4"])

Ideally a private or local value that cannot be injected into the functions domain should be used instead, e.g. (Object.).



 Comments   
Comment by Rick Moynihan [ 12/Jan/18 6:24 AM ]

Apologies for messing up the formatting on the snippet, I also didn't mean to assign this as major. Though I can't seem to edit it, perhaps someone else can.

Comment by Rick Moynihan [ 12/Jan/18 6:39 AM ]

Also thanks to @reborg and @bronsa on #clojure-uk for sharing the above snippet.

Comment by Alex Miller [ 12/Jan/18 7:53 AM ]

Rick - I added you to the necessary groups for edit privileges.

How is this a security risk?

Comment by Rick Moynihan [ 12/Jan/18 7:49 PM ]

Thanks for upping my privileges Alex.

I didn't mean to over egg the pudding, but perhaps I ended up doing it all the same!

My reasoning was that if you're using a transducer to map/reduce over values taken from user input, then an attacker could cause a loop to abort earlier than expected; which might then have other consequences. I did't have a specific attack vector in mind, but I'd seen also the sentinel ::halt and had imagined that a user could inject "clojure.core/halt" into a HTTP header. If ring has a middleware to keywordize keys then you have the sentinel value for a transducer injected by a user, and that sentinel could then potentially be used to shortcut the validation/encoding/escaping etc of other fields.

However I've looked a bit more at the implementation of halt-when and other transducers and it seems the key isn't ever mixed with user data; and that ::none is used to indicate the first pass over by some transducer functions. So it looks like I was mistaken. I doubt now that this is worth resolving.





[CLJ-2311] Spec generator override won't work on multi-spec dispatch key Created: 12/Jan/18  Updated: 12/Jan/18

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

Type: Defect Priority: Minor
Reporter: Andreas Liljeqvist Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: generator, spec
Environment:

[org.clojure/clojure "1.9.0"]
[org.clojure/spec.alpha "0.1.143"]


Attachments: File multi_spec_bug.clj    
Approval: Triaged

 Description   

Overriding generator is not used in the final result for multi-spec dispatch key.

Complete example is attached.
Of particular interest is that the an invalid custom generator can be shown to introduce a failure in validation.
The default generator is seen in the exercised data when using a valid generator.






[CLJ-1189] Map-destructuring :or fumble needs compiler warning Created: 31/Mar/13  Updated: 10/Jan/18  Resolved: 10/Jan/18

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

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

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

 Description   

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

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

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

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

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

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


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

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

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

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

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

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

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

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

Comment by Andy Fingerhut [ 10/Jan/18 8:12 PM ]

Perhaps this ticket can be closed as resolved now that Clojure 1.9.0 is released, and catches these errors?





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

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

Type: Enhancement Priority: Minor
Reporter: Nahuel Greco Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Triaged

 Description   

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

(def ^:mykey a 1)

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

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

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

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

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

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

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


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

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

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

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

Comment by Stuart Halloway [ 31/Jul/15 2:49 PM ]

I am pretty sure we have been here before, and decided that def is working as desired. (If anybody can find the thread/ticket please add a link.) I think this should be a doc enhancement.

If the behavior of defmethod is a separate bug, please make a separate ticket for that, showing an example problem.

Comment by Andy Fingerhut [ 31/Jul/15 3:00 PM ]

CLJ-1213 might be related, but it doesn't mention metadata, only (def foo) without init value given.

Comment by Sean Corfield [ 09/Jan/18 7:33 PM ]

If Stuart is right that this is considered the correct behavior for def then defonce should not use (def ~name) – per CLJ-1148. Could we get a decision once and for all and then reject this issue if the behavior is as designed? Or do we want a patch to update the docstring to make the metadata issue clear?





[CLJ-1148] adds docstring support to defonce Created: 17/Jan/13  Updated: 09/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: Joe Gallo Assignee: Unassigned
Resolution: Unresolved Votes: 12
Labels: docstring

Attachments: Text File 0001-new-defonce-hotness.patch     Text File clj-1148-defonce-2.patch     Text File clj-1148-defonce-3.patch     Text File clj-1148-defonce-4.patch     Text File clj-1148-defonce-4.patch     Text File clj-1148-defonce-5.patch     Text File clj-1148-defonce-6.patch     Text File defonce_fixes.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Pass all args from defonce on to def so it supports docstrings (or potentially other future features) just like def.

Docstrings and other Var metadata will be lost when the defonce is reëvaluated.

Patch: clj-1148-defonce-6.patch

Screened by:



 Comments   
Comment by Alex Miller [ 29/Aug/13 9:53 AM ]

Changed to defect for stomping metadata.

Comment by Stuart Halloway [ 18/Oct/13 8:00 AM ]

Please add tests. The clojure.test-helper namespace has useful temporary namespace support.

Comment by Joe Gallo [ 24/Oct/13 12:44 PM ]

This new patch includes the changes to defonce and also tests.

Comment by Alex Miller [ 24/Oct/13 2:14 PM ]

Changing to Vetted so this is screenable again.

Comment by Rich Hickey [ 22/Nov/13 11:31 AM ]

I disagree about the stomp metadata - different metadata was provided. The purpose of defonce is to avoid the re-evaluation of the init. Is this the simplest change that accomplishes the doc string? In any case split in two.

Comment by Alex Miller [ 29/Dec/13 10:24 PM ]

Reduced scope of ticket to just passing defonce args on to def to add support for docstring. Added new patch that does this.

Comment by Stuart Sierra [ 10/Jan/14 4:09 PM ]

Screened clj-1148-defonce-2.patch but returning to 'incomplete' status.

The :arglists metadata in this patch (a list of symbols) is inconsistent with all other uses of :arglists (a list of vectors).

Other than that the patch is good.

Comment by Alex Miller [ 10/Jan/14 5:04 PM ]

Updated patch to address inconsistency in arglist format and attached clj-1148-defonce-3.patch.

Comment by Stuart Sierra [ 17/Jan/14 9:36 AM ]

The patch clj-1148-defonce-3.patch is OK but it doesn't really address the docstring issue because defonce still destroys metadata. For example:

user=> (defonce foo "docstring for foo" (do (prn 42) 42))
42
#'user/foo
user=> (doc foo)
-------------------------
user/foo
  docstring for foo
nil
user=> (defonce foo "docstring for foo" (do (prn 42) 42))
nil
user=> (doc foo)
-------------------------
user/foo
  nil
Comment by Stuart Sierra [ 17/Jan/14 10:03 AM ]

Screened with reservations noted.

Comment by Rich Hickey [ 24/Jan/14 10:15 AM ]

Stuart is right, second defonce should retain the doc string (since it again provides it, should be no-op)

Comment by Alex Miller [ 20/Feb/14 10:41 AM ]

pull out of 1.6

Comment by Linus Ericsson [ 28/Aug/14 12:30 PM ]

This version looks for previously defined var with resolve. A repeated defonce won't affect the namespace at all if the variable is already defined and bounded.

Please confirm using (resolve '~name) is not a problem w.r.t ns-bindings or similar.

This patch also contains the tests from clj-1148-defonce-3.patch as well as the :arglists property.

(patch 4 missed one def-row, sorry for mailbox noise).

Comment by Linus Ericsson [ 09/Sep/14 4:27 AM ]

Yet another, simpler version of defonce. No test-cases included.

This version just makes an (or (nil? v#) (not (.hasRoot v#)) test on the resolved variable. If this is true, really define by (def ~name ~@args) else do nothing.

Comment by Andy Fingerhut [ 08/Jan/15 6:04 PM ]

Linus, while JIRA can handle multiple attachments on the same ticket with identical names, it can be confusing to do so. Would you mind renaming or removing the ones you have added with duplicate names, e.g. delete obsolete ones, perhaps? Instructions for deleting patches are in the "Removing patches" section on this wiki page: http://dev.clojure.org/display/community/Developing+Patches

Comment by Sean Corfield [ 09/Jan/18 7:30 PM ]

On noticing that we have a JIRA issue at work to add docstrings to the handful of defonce calls we have (via alter-meta!), I came upon this issue and wondered what is stopping it moving forward?

Is Linus's patch an acceptable solution overall (and is just missing tests)? Or is a different approach wanted? Is this blocked by CLJ-1446 and folks want that fixed before updating defonce?





[CLJ-2309] Readable keyword validation Created: 05/Jan/18  Updated: 08/Jan/18

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

Type: Enhancement Priority: Minor
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: keywords

Approval: Triaged

 Description   

A common question is why it is possible to programatically create keyword instances that cannot be read (https://clojure.org/guides/faq#unreadable_keywords).

a) There are many places where programmatic keywords are perfectly valid
b) It is undesirable to validate all keywords on construction due to the perf hit

However, there are use cases for creating keywords that you expect to be safe for pr/read roundtripping and in those cases it would be useful to have either a variant of `keyword` that was more restrictive and would throw on invalid keywords OR a predicate that could tell you whether a string would be a readable keyword.



 Comments   
Comment by Phill Wolf [ 08/Jan/18 7:56 PM ]

1. Keywords are too often prepared far away from the eventual pr.

2. It is impressive how seldom deviant keywords are a problem this way. The fix should be proportionate to the problem.

3. Irreadable keywords are explicitly not the problem. The problem belongs to pr and read. The immediate problem is that pr silently fails to print readably while print-readably.

3b. This problem with pr/read might be temporary.

4. Adding a special predicate for readable keywords presupposes that you can predict whether someone will call later pr. I think such a prediction would usually be baseless without mingling concerns.

5. pr could increment irreadable-forms-printed when emitting a non-readable keyword while print-readably. I/O is expensive anyway.

6. If pr/read someday gained the ability to round-trip all keywords, pr would still have to test a keyword to choose an output format for it. Thus a check installed today could be a first step, not a temporary measure.

7. By comparison, a program littered with "readable-keyword?" predicates would stay littered forever – and the predicate check might not even be necessary anymore.

8. Would "readable-keyword?" match the reader's behavior, the reader's documentation, EDN, or some conservative common subset? related: CLJ-1527 "Clarify and align valid symbol and keyword rules for Clojure (and edn)", CLJ-1530 "Make foo/bar/baz unreadable".

9. Would every spec writer agonize about whether to spec a keyword as "keyword?" or "readable-keyword?" – and then always choose "readable-keyword?" just in case someone might ever someday eventually decide to serialize data?.. thereby cluttering the program and obscuring concerns.

10. Who would test irreadable-forms-printed? An "apologetic", non-solution resolution could get quite complicated.

11. Perhaps the cure would be worse than the disease. Why not improve pr/read instead: eliminate the problem instead of adding features to work around it?

Comment by Alex Miller [ 08/Jan/18 8:45 PM ]

The problem here is specifically about keywords created from either user input or other data outside your control (arbitrary keys from json input for example). When you need to verify this property you are accepting inputs, converting them to keywords, and then later expect to print that data out. I have talked through this with many many people and when people ask about it, they know they are in this situation. Having printing fail or report way down the line is not helpful. The problem is avoiding the creation of the non-roundtrippable data in the first place (and either not accepting it or escaping it at that point).

That said, another possible solution is to add an escaping mechanisms for literal symbols and keywords. We've done some design work on this in the past and ended up shelving it at the time but that's still another possible option.

This is not a high priority issue right now, but I felt it was useful to leave this ticket here to capture the idea.





[CLJ-2310] Note that file-seq doesn't guarantee file order Created: 07/Jan/18  Updated: 07/Jan/18

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

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

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

 Description   

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



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

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

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

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

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





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

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

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


 Description   

Hi,

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

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

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

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

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

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

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

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

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

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



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

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

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

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

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

Hello Andy,

thank your for taking the time.

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

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

Why I filed this issue:

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

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

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

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

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

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

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

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

I expect there to be many more newbies to clojure

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

Greetings and have a formidable 2018!

Markus

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

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





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

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

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


 Description   

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

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

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

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

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

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



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

Duplicate with CLJ-1257 ?

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

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

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

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

Comment by James Craig Burley [ 04/Jan/18 6:03 PM ]

Another use case (of possibly questionable validity) is defining a function or macro that has the same name as something in clojure.core but is not intended as a drop-in replacement, rather something else entirely.

E.g. in my early efforts to learn Clojure, I'm trying out some coding examples and want to run (and time) various test cases against different versions of a function that is spec'ed to read from *in*. So I threw together a quick little bash-shell-like namespace defining macros named <, >, and >>, to do the "obvious" things (invoke the body of code with pertinent globals bound to a suitably opened file). Use case is something like:

(require ['com.burleyarch.bash :as bash])
(bash/< "path-to-input-file" (read-line))

It'd be nice to avoid warnings for that case as well, without anyone assuming the function/macro is intended as a "drop-in replacement" for any namespace.

(Yes, I realize that what I'm trying to do here is probably unsuitable for general consumption and/or duplicative of something already available....)





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

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

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

all


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

 Description   

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

Alex suggested it be removed in this discussion;

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

The attached patch removes rev from Var.

Prescreened: Alex Miller






[CLJ-2305] float casting not found in map Created: 03/Jan/18  Updated: 03/Jan/18

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

Type: Defect Priority: Major
Reporter: Asher Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: casting, float


 Description   

When getting the value of a floated number in a map, the value isn't found if the map has more than 8 values.

For example:

(get {1.0 "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h" 1 "i"} (float 1)) => nil
(get {1.0 "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h"} (float 1)) => "a"



 Comments   
Comment by Asher [ 03/Jan/18 3:13 AM ]

The problem isn't only with maps.

Here is an example with a Set:
(contains? #{1.0 1.1} (float 1)) => true
(contains? #{1.0 1.1 1.2} (float 1)) => false

Comment by Andy Fingerhut [ 03/Jan/18 4:03 AM ]

I believe that the fact that get lookup of a float in a map with at most 8 keys is an accident of the implementation.

The reason why it fails with larger maps and hash sets is that the hash value is different for (double 1.0) and (float 1.0).

user=> (hash (float 1.0))
1065353216
user=> (hash 1.0)
1072693248

All of the values like 1.0 and others in your example default to type double. Note that if you force the keys of a map to be float, then lookup succeeds, even if the map has more than 8 keys:

user=> (get {(float 1.0) "a" 2.0 "b" 3.0 "c" 4.0 "d" 5.0 "e" 6.0 "f" 7.0 "g" 8.0 "h" 1 "i"} (float 1)) 
"a"

My guess is that except for the accident that get with a small map happens to return a successful lookup, everything here is working as designed.

I believe the reason that get with a small map does find a match for looking up (float 1) is probably because the implementation of small maps is as an ArrayMap, where lookup keys are sequentially compared against all keys in an array using clojure.core/= (or its Java equivalent), and:

user=> (= (float 1) (double 1))
true
Comment by Andy Fingerhut [ 03/Jan/18 4:10 AM ]

Additional note: CLJ-1649 was created by someone hoping that floats and doubles should have hash and = consistent with each other. You can read more about it on that ticket's description. No judgment has been made whether such a change will ever be made in Clojure, but if one of Rich's comments on ticket CLJ-1036 is any indication, it seems unlikely to change.

General suggestion: Don't mix floats and doubles in Clojure code if you can possibly avoid it.

Comment by Asher [ 03/Jan/18 4:20 AM ]

Thank you @Andy!





[CLJ-2304] CLONE - spec passing nil as second argument to `ExceptionInfo` constructor... Created: 02/Jan/18  Updated: 02/Jan/18

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

Type: Defect Priority: Major
Reporter: Jonathan Leonard Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bug, spec
Environment:

Mac OS X 10.13.2

jonathan$ java -version
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)


Attachments: Text File better-messaging.patch    

 Description   

Exception thrown from this line:
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/ExceptionInfo.java#L31

Due to this call:
https://github.com/clojure/spec.alpha/blob/master/src/main/clojure/clojure/spec/test/alpha.clj#L279

if `when-not` returns nil, `ExceptionInfo` throws exception on line 31.

A naive fix may be:
(apply ex-info (remove nil? ["Specification-based check failed" (when-not ...)]))

although that is really just masking over the issue and provides no actionable info to the sufferer of the failure.

Unfortunately I have no minimal test case as this is proprietary (and complicated code).

Perhaps the author from reading the code will have more insight.

Here's the full stack trace:

[[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 31]
[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 22]
[clojure.core$ex_info invokeStatic "core.clj" 4739]
[clojure.core$ex_info invoke "core.clj" 4739]
[clojure.spec.test.alpha$explain_check invokeStatic "alpha.clj" 277]
[clojure.spec.test.alpha$explain_check invoke "alpha.clj" 275]
[clojure.spec.test.alpha$check_call invokeStatic "alpha.clj" 295]
[clojure.spec.test.alpha$check_call invoke "alpha.clj" 285]
[clojure.spec.test.alpha$quick_check$fn__2986 invoke "alpha.clj" 308]
[clojure.lang.AFn applyToHelper "AFn.java" 154]
[clojure.lang.AFn applyTo "AFn.java" 144]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.test.check.properties$apply_gen$fn__16139$fn__16140 invoke "properties.cljc" 30]
[clojure.test.check.properties$apply_gen$fn__16139 invoke "properties.cljc" 29]
[clojure.test.check.rose_tree$fmap invokeStatic "rose_tree.cljc" 77]
[clojure.test.check.rose_tree$fmap invoke "rose_tree.cljc" 73]
[clojure.test.check.generators$fmap$fn__9199 invoke "generators.cljc" 101]
[clojure.test.check.generators$gen_fmap$fn__9173 invoke "generators.cljc" 57]
[clojure.test.check.generators$call_gen invokeStatic "generators.cljc" 41]
[clojure.test.check.generators$call_gen invoke "generators.cljc" 37]
[clojure.test.check$quick_check invokeStatic "check.cljc" 94]
[clojure.test.check$quick_check doInvoke "check.cljc" 37]
[clojure.lang.RestFn invoke "RestFn.java" 425]
[clojure.lang.AFn applyToHelper "AFn.java" 156]
[clojure.lang.RestFn applyTo "RestFn.java" 132]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.gen.alpha$quick_check invokeStatic "alpha.clj" 29]
[clojure.spec.gen.alpha$quick_check doInvoke "alpha.clj" 27]
[clojure.lang.RestFn applyTo "RestFn.java" 137]
[clojure.core$apply invokeStatic "core.clj" 661]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.test.alpha$quick_check invokeStatic "alpha.clj" 309]
[clojure.spec.test.alpha$quick_check invoke "alpha.clj" 302]
[clojure.spec.test.alpha$check_1 invokeStatic "alpha.clj" 335]
[clojure.spec.test.alpha$check_1 invoke "alpha.clj" 323]
[clojure.spec.test.alpha$check$fn__3005 invoke "alpha.clj" 411]
[clojure.core$pmap$fn__8105$fn__8106 invoke "core.clj" 6942]
[clojure.core$binding_conveyor_fn$fn__5476 invoke "core.clj" 2022]
[clojure.lang.AFn call "AFn.java" 18]
[java.util.concurrent.FutureTask run "FutureTask.java" 266]
[java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1149]
[java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 624]
[java.lang.Thread run "Thread.java" 748]]



 Comments   
Comment by Jonathan Leonard [ 02/Jan/18 12:47 PM ]

So, I was able to finally track this down to an actual problem where my function value returned from the function under test would throw and exception on a particular input.

This patch helps point people in that direction (although an even better one would be to surface/explain the underlying failure rather than just allude to it).





[CLJ-1891] New socket server startup proactively loads too much code, slowing boot time Created: 09/Feb/16  Updated: 02/Jan/18

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

Type: Enhancement Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: server

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

 Description   

In the new socket server code, clojure.core.server is proactively loaded (regardless of whether servers are in the config), which will also load clojure.edn and clojure.string.

Approach: Delay loading of this code until the first server config is found. This improves startup time when not using the socket server about 0.05 s.

Patch: clj-1891.patch



 Comments   
Comment by Alexander Yakushev [ 02/Jan/18 9:17 AM ]

Bump. With the introduction of Spec, and considering the fact that clojure.core.server triggers the initialization of Spec, the benefit of solving this issue should now be bigger than 0.05 sec (more like 0.2 sec). See http://clojure-goes-fast.com/blog/clojures-slow-start .





[CLJ-2303] Reified object doesn't satisfy protocol at compile time Created: 29/Dec/17  Updated: 02/Jan/18

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

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


 Description   

https://github.com/MichaelBlume/reify-fail

(defprotocol Foo
  (hello [this] "Says hello"))

(def bar
  (reify Foo
    (hello [this] "hello, world")))

(when-not (satisfies? Foo bar)
  (throw (Exception. "bar doesn't satisfy Foo")))

Notes:

Project is AOT
Another namespace requires this namespace. That namespace is compiled first. Without that namespace the problem goes away.



 Comments   
Comment by Kevin Downey [ 01/Jan/18 5:15 PM ]

what steps do you take after checking out the repo to reproduce this issue?

I cloned the repo and ran `lein compile` and didn't get any errors.

Comment by Michael Blume [ 01/Jan/18 10:56 PM ]

I was reproducing the issue with `lein check`, but `lein compile` also fails for me, so I'm confused.

Comment by Michael Blume [ 01/Jan/18 10:56 PM ]

I think it's important that the near-empty namespace compiles first, maybe there are platform differences in compilation order?

Comment by Michael Blume [ 01/Jan/18 10:57 PM ]

test repo fails for me under Mac OS and Ubuntu, both with Java 8

Comment by Michael Blume [ 01/Jan/18 10:58 PM ]

Under Ubuntu I have

$ lein -v
Leiningen 2.7.1 on Java 1.8.0_151 OpenJDK 64-Bit Server VM

Comment by Andy Fingerhut [ 02/Jan/18 12:19 AM ]

On both of these OS’s: Ubuntu 16.04.3 Linux, macOS 10.12.6

With both of these versions of Leiningen: 2.7.1, 2.8.1

I believe all of my experiments were using a recent version of JDK 1.8

With any of these Clojure versions: 1.7.0 1.8.0 1.9.0

I get an error with both the commands 'lein check' and 'lein compile' with the project https://github.com/MichaelBlume/reify-fail, but only if I do '/bin/rm -fr target' first (or run it before the target directory has ever been created, e.g. immediately after running the ‘git clone’ command for the repository).

If I run the command twice, the first command creates many .class files in the target directory, and the second such command gives no error.

Comment by Kevin Downey [ 02/Jan/18 12:36 AM ]

I am able to reproduce there error if I replace `:aot :all` with `:aot [com.lendup.citadel.pipeline com.lendup.citadel.providers.mocky]`. I suspect lein could fix this by doing their stale ns check between compiling namespaces to avoid repeated compilation of transitive namespaces.

Comment by Kevin Downey [ 02/Jan/18 12:41 AM ]

could be related to https://github.com/technomancy/leiningen/issues/2316





[CLJ-2287] Add clojure.set/union, intersection, difference to core.spec.alpha Created: 12/Dec/17  Updated: 30/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJ-2287-add-clojure-set-specs-v1.patch    

 Description   

Several tickets have been created suggesting that several functions in the clojure.set namespace might throw an exception if given non-sets as arguments, because in some cases they return unexpected values. This list is a sample, not intended to be complete: CLJ-810, CLJ-1682, CLJ-1953, CLJ-1954

Now that clojure.spec exists, documenting the expected argument types of these functions can be done more precisely. The specs could be used to dynamically detect incorrect argument types passed to these functions during testing, and via any other ways built on top of clojure.spec in the future.

Alex Miller suggested in a Slack discussion that perhaps it would be helpful if contributors would help implement such specs for functions within Clojure.

The approach taken with the proposed patch is simply to spec that the :args of clojure.set/subset?, superset?, union, intersection, and difference must all be sets, and spec the :ret types of the first two as boolean, and the last three as set. This seems to match all known comments given when declining previous tickets proposing changes to the behavior of these functions.

Patch: *CLJ-2287-add-clojure-set-specs-v1.patch*

I (Andy Fingerhut) looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can correct this (assuming it is considered a bug) on this ticket: CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.

Please suggest any other ways that these changes could be evaluated in order to increase your confidence that they are correct.



 Comments   
Comment by Alex Miller [ 12/Dec/17 1:37 PM ]

For the moment I’ll say sure. I think this particular example is a good one.

I’m not sure that all the specs here are as obvious as you expect, or at least that’s been my experience elsewhere.

I’d expect these specs to be in clojure.set.specs namespace btw.

Comment by Andy Fingerhut [ 13/Dec/17 4:33 PM ]

Attached CLJ-2287-add-clojure-set-specs-v1.patch dated 2017-Dec-13. These are my first Clojure specs ever, so feel free to go crazy on suggesting improvements.

I used the namespace clojure.set.specs.alpha rather than clojure.set.specs as you suggested, but I can easily change that if you really did want it without the .alpha

Comment by Andy Fingerhut [ 13/Dec/17 4:34 PM ]

If there is some way I can exercise the :args specs with a large suite of tests, let me know how and I can try it.

And clearly I am putting in at least for :args and :ret pretty obvious choices of specs that I believe are correct, given issues raised against those functions in the past. I don't see how they could be any more subtle than that for these functions, but let me know if I'm missing anything, e.g. the Clojure core team has decided to support non-set arguments for these functions.

I agree that some of the other functions in the clojure.set namespace might be a bit trickier to spec the args of.

Comment by Andy Fingerhut [ 14/Dec/17 8:33 PM ]

I looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can 'correct' this (assuming it is considered a bug) on this ticket: https://dev.clojure.org/jira/browse/CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.





[CLJ-2302] s/merge fails with plain predicate Created: 29/Dec/17  Updated: 29/Dec/17

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

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

[org.clojure/spec.alpha "0.1.143"]



 Description   

I would expect a plain predicate work with s/merge:

(s/explain any? {:x 1, :y 2})
; Success!

(s/explain (s/merge any?) {:x 1, :y 2})
; val: {:x 1, :y 2} fails predicate: any?

Still, wrapping the predicate into specs seems to work:

(s/explain (s/merge (s/spec any?)) {:x 1, :y 2})
; Success!





[CLJ-2301] 'ensure' can cause livelock Created: 26/Dec/17  Updated: 26/Dec/17

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

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


 Description   

The following program demonstrates livelock when using ensure.

(let [cats (ref 1)
      dogs (ref 1)
      events (atom [])
      john (future
             (dosync
               (swap! events conj :j1)
               (when (< (+ @cats (ensure dogs)) 3)
                 (swap! events conj :j2)
                 (ref-set cats 2))
               (swap! events conj :j3)))
      mary (future
             (dosync
               (swap! events conj :m1)
               (when (< (+ (ensure cats) @dogs) 3)
                 (swap! events conj :m2)
                 (ref-set dogs 2))
               (swap! events conj :m3)))]
  @john @mary @events)
; => [:m1 :j1 :m2 :j2 :j1 :m1 :j2 :m2 :j1 :m1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :j2 :m2 :j1 :m1 :j2 :m2 :m1 :j1 :m2 :j2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :m1 :j1 :j2 :m2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :j1 :m2 :j2 :j1 :m1 :j2 :m2 :m1 :j1 :m2 :j2 :j1 :m1 :m2 :j2 :m1 :j1 :j2 :m2 :m1 :j1 :m2 :j2 :m1 :m2 :j1 :m3 :j1 :j3]

This program sometimes terminates very quickly, but sometimes there is a delay of several seconds as the transactions are retried dozens or hundreds of times.

(ensure ref) can exhibit livelock in a way that the equivalent (ref-set ref @ref) cannot, because only the latter supports barging. With ref-set older transactions will ultimately win thanks to barging. With ensure there is no barging as both transactions try to get the write lock (ref-set) of the ref whose read lock is held by the other transaction (ensure). Both transactions sit idle for the full 100ms timeout and will then simultaneously retry.

This could be just a doc enhancement. The sentence 'Allows for more concurrency than (ref-set ref @ref)' may not be true generally (and is anyway too vague to be much help). (ref-set ref @ref) is safer than (ensure ref).



 Comments   
Comment by David Bürgin [ 26/Dec/17 4:43 AM ]

From an old mailing list discussion here and here.





[CLJ-2284] Incorrect bytecode generated for static methods on interfaces Created: 09/Dec/17  Updated: 24/Dec/17

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

Type: Defect Priority: Major
Reporter: Zach Tellman Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: asm, compiler, java19
Environment:

JDK 9 or higher


Attachments: Text File 0001-unbundle-ASM.patch     Text File 0002-update-classfile-version-to-9.patch     Text File 0003-CLJ-2284.patch    
Approval: Triaged

 Description   

A reproduction of the failure can be found here: https://github.com/ztellman/java9-failure.



 Comments   
Comment by Ghadi Shayban [ 09/Dec/17 11:08 PM ]

Static or default interface method invocations are not supported when the bytecode emitted is < 1.8 bytecode (Clojure currently emits 1.6 bytecode). I'm mildly surprised this compiled.

Comment by David Bürgin [ 21/Dec/17 12:54 PM ]

Not sure Ghadi’s assertion is correct? No new bytecodes were introduced for static and default interface methods. Or at least I have been using JDK 8 utilities like CharSequence.codePoints() and Comparator.reverseOrder() in Clojure for a long time. (And if this usage were not supported today that would seem like a critical issue.)

Comment by Zach Tellman [ 22/Dec/17 9:08 PM ]

Yeah, I'd expect default interface methods to just be filled in on classes which don't provide their own implementation, and empirically they don't seem to cause any issues, but I'm not an authority on JVM bytecode.

Comment by Nicola Mometto [ 23/Dec/17 2:10 PM ]

while it is true that no bytecodes were introduced, the JVM spec mandates that invocations to static interface methods refer to an InterfaceMethodref entry in the constant pool rather than simply to a Methodref – as to why this worked in jvm 1.8 and fails with a verify error in jvm 9 , I can only assume the bytecode verifier became stricter since version 9

See https://docs.oracle.com/javase/specs/jvms/se9/html/jvms-5.html#jvms-5.4.3.3, describing the resolution rules for Methodrefs:

When resolving a method reference:

    If C is an interface, method resolution throws an IncompatibleClassChangeError.
Comment by Nicola Mometto [ 23/Dec/17 2:21 PM ]

Indeed here's the openjdk ticket tightening bytecode verification to adhere to the JVM spec: https://bugs.openjdk.java.net/browse/JDK-8145148
According to the last comment on that ticket, upgrading the ASM version provided with clojure to 5.1 should fix this

Comment by Nicola Mometto [ 24/Dec/17 12:31 PM ]

The following 3 patches work to fix this problem, I've kept them split for now so that we can be evaluated separately, but in order to fix this all three are required.

1st patch simply updates the version of ASM that clojure uses to 6.0, removes the bundled ASM and adds ASM as a separate dependency. If this is not desirable we could just update the bundled version of ASM but it seems like now that clojure requires external deps, this is up for debate.

2nd patch updates the classfile version that clojure emits from 1_5 to 9 so that ASM can emit interface method refs, because of changes in bytecode verification since v1.5, several additional changes were needed:
1- it's not possible anymore to assign final fields from methods other than <clinit>, so it was necessary to mark the const__ fields as non static as they are assigned by init__n methods
2- stack map frames are not optional anymore so we need to tell ASM to compute them for us
3- a custom ClassWriter overriding getCommonSuperClass has been implemented, using DCL to load classes and short circuiting on classes that are being defined (as per that method's javadoc)

finally the third patch addresses this issue by emitting `invokeStatic` through the new `visitmethodInsn` arity that supports interface methods

Comment by Nicola Mometto [ 24/Dec/17 12:41 PM ]

Note that I'm not proposing the 3 patches as they are as a fix for this, given that bumping classfile version means discontinuing support for running clojure on java versions earlier than what's specified, which is undesiderable (I've bumped it to V9 but what's needed here should be just V1_8, but still).

That said, if we want to fix this, we definitely need the fixes I've put in those patches and we likely also want to conditionally emit different classfile versions (e.g. defaulting to 1_5 but bumping it to a higher version for specific classfiles when needed)





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

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

Type: Defect Priority: Major
Reporter: Jonathan Leonard Assignee: Unassigned
Resolution: Not Reproducible Votes: 0
Labels: bug, spec
Environment:

Mac OS X 10.13.2

jonathan$ java -version
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)



 Description   

Exception thrown from this line:
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/ExceptionInfo.java#L31

Due to this call:
https://github.com/clojure/spec.alpha/blob/master/src/main/clojure/clojure/spec/test/alpha.clj#L279

if `when-not` returns nil, `ExceptionInfo` throws exception on line 31.

A naive fix may be:
(apply ex-info (remove nil? ["Specification-based check failed" (when-not ...)]))

although that is really just masking over the issue and provides no actionable info to the sufferer of the failure.

Unfortunately I have no minimal test case as this is proprietary (and complicated code).

Perhaps the author from reading the code will have more insight.

Here's the full stack trace:

[[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 31]
[clojure.lang.ExceptionInfo <init> "ExceptionInfo.java" 22]
[clojure.core$ex_info invokeStatic "core.clj" 4739]
[clojure.core$ex_info invoke "core.clj" 4739]
[clojure.spec.test.alpha$explain_check invokeStatic "alpha.clj" 277]
[clojure.spec.test.alpha$explain_check invoke "alpha.clj" 275]
[clojure.spec.test.alpha$check_call invokeStatic "alpha.clj" 295]
[clojure.spec.test.alpha$check_call invoke "alpha.clj" 285]
[clojure.spec.test.alpha$quick_check$fn__2986 invoke "alpha.clj" 308]
[clojure.lang.AFn applyToHelper "AFn.java" 154]
[clojure.lang.AFn applyTo "AFn.java" 144]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.test.check.properties$apply_gen$fn__16139$fn__16140 invoke "properties.cljc" 30]
[clojure.test.check.properties$apply_gen$fn__16139 invoke "properties.cljc" 29]
[clojure.test.check.rose_tree$fmap invokeStatic "rose_tree.cljc" 77]
[clojure.test.check.rose_tree$fmap invoke "rose_tree.cljc" 73]
[clojure.test.check.generators$fmap$fn__9199 invoke "generators.cljc" 101]
[clojure.test.check.generators$gen_fmap$fn__9173 invoke "generators.cljc" 57]
[clojure.test.check.generators$call_gen invokeStatic "generators.cljc" 41]
[clojure.test.check.generators$call_gen invoke "generators.cljc" 37]
[clojure.test.check$quick_check invokeStatic "check.cljc" 94]
[clojure.test.check$quick_check doInvoke "check.cljc" 37]
[clojure.lang.RestFn invoke "RestFn.java" 425]
[clojure.lang.AFn applyToHelper "AFn.java" 156]
[clojure.lang.RestFn applyTo "RestFn.java" 132]
[clojure.core$apply invokeStatic "core.clj" 657]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.gen.alpha$quick_check invokeStatic "alpha.clj" 29]
[clojure.spec.gen.alpha$quick_check doInvoke "alpha.clj" 27]
[clojure.lang.RestFn applyTo "RestFn.java" 137]
[clojure.core$apply invokeStatic "core.clj" 661]
[clojure.core$apply invoke "core.clj" 652]
[clojure.spec.test.alpha$quick_check invokeStatic "alpha.clj" 309]
[clojure.spec.test.alpha$quick_check invoke "alpha.clj" 302]
[clojure.spec.test.alpha$check_1 invokeStatic "alpha.clj" 335]
[clojure.spec.test.alpha$check_1 invoke "alpha.clj" 323]
[clojure.spec.test.alpha$check$fn__3005 invoke "alpha.clj" 411]
[clojure.core$pmap$fn__8105$fn__8106 invoke "core.clj" 6942]
[clojure.core$binding_conveyor_fn$fn__5476 invoke "core.clj" 2022]
[clojure.lang.AFn call "AFn.java" 18]
[java.util.concurrent.FutureTask run "FutureTask.java" 266]
[java.util.concurrent.ThreadPoolExecutor runWorker "ThreadPoolExecutor.java" 1149]
[java.util.concurrent.ThreadPoolExecutor$Worker run "ThreadPoolExecutor.java" 624]
[java.lang.Thread run "Thread.java" 748]]



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

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

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

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

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





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

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

Type: Defect Priority: Major
Reporter: Jonathan Leonard Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: bug, spec
Environment:

Mac OS X 10.13.2

jonathan$ java -version
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)



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


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

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

Use this:

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

Invalid function implementation

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

Ah, thank you!

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

[left a comment there requesting the update]

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

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

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

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





[CLJ-2298] Calling static interface methods in Java 9 causes error Created: 21/Dec/17  Updated: 21/Dec/17

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

Type: Defect Priority: Major
Reporter: Piotr Bzdyl Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, java9
Environment:

Java 9.0.1+11
Clojure 1.9.0



 Description   

The byte code generated for calls to a static interface method when running with Java 9 fails with:

{{java.lang.IncompatibleClassChangeError: Method [.....]()L[......]; must be InterfaceMethodref constant}}

It can be recreated with:

public interface Foo {
  public static String bar() {
    return "test";
  }
}

javac Foo.java

And then calling the method from Clojure namespace results in:

java -cp clojure-1.9.0.jar:spec.alpha-0.1.143.jar:spec.alpha-0.1.143.jar:. clojure.main -e (Foo/bar)

Exception in thread "main" java.lang.IncompatibleClassChangeError: Method Foo.bar()Ljava/lang/String; must be InterfaceMethodref constant
	at user$eval11.invokeStatic(NO_SOURCE_FILE:1)
	at user$eval11.invoke(NO_SOURCE_FILE:1)
	at clojure.lang.Compiler.eval(Compiler.java:7062)
	at clojure.lang.Compiler.eval(Compiler.java:7025)
	at clojure.core$eval.invokeStatic(core.clj:3206)
	at clojure.main$eval_opt.invokeStatic(main.clj:291)
	at clojure.main$eval_opt.invoke(main.clj:285)
	at clojure.main$initialize.invokeStatic(main.clj:311)
	at clojure.main$null_opt.invokeStatic(main.clj:345)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)

I think it might be related to an issue in asm that has been already fixed but I didn't have enough time to verify this.



 Comments   
Comment by David Bürgin [ 21/Dec/17 12:53 PM ]

Duplicate of CLJ-2284





[CLJ-2297] PersistentHashMap leaks memory when keys are removed with `without` Created: 20/Dec/17  Updated: 20/Dec/17

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

Type: Defect Priority: Critical
Reporter: Ben Bader Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: collections

Attachments: File demo-phm-leak.clj     Text File fix-bitmapnode-without.patch    
Approval: Triaged

 Description   

The problem is in `PersistentHashMap.BitmapNode#without(Object)`; when the last value is removed, an empty BitmapNode is returned instead of null. This has knock-on effects in large maps that have interior ArrayNodes, which themselves are not preserved.

Attached is a test script that demonstrates the issue, by measuring heap usage, adding a large number of entries to a map, removing those keys one-by-one, then comparing heap usage afterwards. The output, shows the average amount of heap allocated for an empty map that has had 100,000 entries added then removed:

❯ java -jar clojure.jar demo-phm-leak.clj
Avg. heap:8656352/5 bytes
Avg. heap:8656632/5 bytes
Avg. heap:8654664/5 bytes
Avg. heap:8656884/5 bytes
Avg. heap:1731156 bytes

This was from my a Macbook Pro, running a self-made Clojure built from master, using java 1.8.0_65. The variable sizes are due to the fact that the shape of the PHM tree depends on the insertion order of keys, which is randomized in this script.



 Comments   
Comment by Ben Bader [ 20/Dec/17 1:33 PM ]

This patch fixes the leak by changing `BitmapNode#without(Object)` such that, when there are no other values in the node, the method returns `null` instead of an empty BitmapNode.

Output from the demo script, from a Clojure build incorporating the patch:

~/Development/clojure collapse-empty-mapnodes* 16s
❯ java -jar clojure.jar demo-phm-leak.clj
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes
Avg. heap:64 bytes

Comment by Ben Bader [ 20/Dec/17 1:54 PM ]

Note that this patch doesn't have a similar change for the transient overload of `without`; because it relies on a helper method `editAndRemovePair` that correctly handles the empty case, that method doesn't have this bug.

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

Thanks for the report!

Comment by Alex Miller [ 20/Dec/17 2:23 PM ]

I assume this came out of some actual usage with surprising behavior?

Comment by Ben Bader [ 20/Dec/17 4:44 PM ]

It came up when I was profiling a service that uses maps in agents to cache things; overall memory usage seemed high, and I got curious. Hard to say whether this would ever noticeably impact a production system, given that this behavior has been in place for nine years!

Comment by Andy Fingerhut [ 20/Dec/17 7:34 PM ]

Cool find. It looks like an average of 1.7 bytes of unreclaimed space per 'without' operation in your test case, so nice to improve upon, but perhaps not something people would notice due to it causing problems very often.





[CLJ-2296] Refactoring vec function Created: 19/Dec/17  Updated: 20/Dec/17  Resolved: 20/Dec/17

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

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


 Description   

vec function could be re-written like this:

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



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

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

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

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

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

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

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

Hi Alex and Nicola,

Fair enough. Thanks for the feedback!





[CLJ-2293] Compiler allows duplicated function parameter names Created: 14/Dec/17  Updated: 17/Dec/17  Resolved: 15/Dec/17

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

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

MacOS, Java 8



 Description   

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

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

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



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

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

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

this is intentional, it works just like normal let shadowing

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

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

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

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

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

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

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

Thanks!

Matt.

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

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

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

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

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

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

Anyway, moving on. Thanks for the feedback.





[CLJ-2295] clojure.repl/doc repeats doc string in output for special forms in Clojure 1.9.0 Created: 15/Dec/17  Updated: 15/Dec/17

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

Type: Defect Priority: Minor
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJ-2295-v1.patch    

 Description   

Here is output from Clojure 1.8.0:

user=> (doc if)
-------------------------
if
  (if test then else?)
Special Form
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.

  Please see http://clojure.org/special_forms#if
nil

Here is the corresponding output from Clojure 1.9.0:

user=> (doc if)
-------------------------
if
  (if test then else?)
Special Form
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.

  Please see http://clojure.org/special_forms#if
  Evaluates test. If not the singular values nil or false,
  evaluates and yields then, otherwise, evaluates and yields else. If
  else is not supplied it defaults to nil.
nil

This repetition only occurs when calling clojure.repl/doc or clojure.repl/print-doc for special form symbols, not for other symbols like macros and functions. It was introduced when modifying clojure.repl/print-doc when adding the ability to print specs in its output, and the fix is straightforward.



 Comments   
Comment by Andy Fingerhut [ 15/Dec/17 6:20 PM ]

Patch CLJ-2295-v1.patch dated 2017-Dec-15 is one possible way to fix this issue. Verified that output for other cases, e.g. macros, is unchanged from Clojure 1.8.0 with these changes, except for the new Spec output, which should be kept.





[CLJ-2283] doseq should return nil with no collections Created: 09/Dec/17  Updated: 15/Dec/17

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

Type: Defect Priority: Major
Reporter: Ambrose Bonnaire-Sergeant Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File CLJ-2283.patch     Text File CLJ-2283-test.patch     Text File CLJ-2283-test-revised.patch    
Approval: Triaged

 Description   

According to the docstring for doseq, the following should return `nil`.

user> (doseq [] "not nil")
"not nil"


 Comments   
Comment by Michael Zavarella [ 13/Dec/17 3:47 PM ]

I think this fixes this issue.

Theres a `(do ...)` that runs if `(seq exprs)` is nil but the return of that `do` isn't necessarily nil if your body is something that doesn't return nil.

Comment by Mike Fikes [ 14/Dec/17 2:12 PM ]

It is odd that doseq even has code for this case given that the docstring indicates the bindings and filterings are as provided by "for." Since for requires one or more binding forms, it raises the question of whether (doseq [] "not nil") is a valid program.

Comment by Alex Miller [ 14/Dec/17 2:58 PM ]

Test?

Comment by Michael Zavarella [ 15/Dec/17 5:34 PM ]

Here's a bit of tests! I think these are suitable for the use case of `doseq`. I just went with similar tests to `dotimes` since they're so similar.

Comment by Michael Zavarella [ 15/Dec/17 5:36 PM ]

I removed the `println` from the original tests in favor of `identity`. It seemed more appropriate so you don't have a random 'foo' in the test print-out.





[CLJ-2294] Allow user to get forms of multi-spec implementations Created: 14/Dec/17  Updated: 14/Dec/17

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

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

org.clojure/spec.alpha "0.1.143"



 Description   

As noted in this thread (https://groups.google.com/forum/#!topic/clojure/i8Rz-AnCoa8),

s/keys
does not validate that the specs mentioned within it are valid specs.

A suggested workaround is to separately write code that could inspect the registry and warn about specs that are not declared. A sketch of this code was provided at https://gist.github.com/stuarthalloway/f4c4297d344651c99827769e1c3d34e9

While the general approach is a good one, I believe it won't work for multi-specs that contain "keys" specs.

s/form
on a multi-spec will only return that the spec is a multi-spec (which makes sense), but there is no way for a client to get access to code that picks the approach spec given data.

A further workaround is provided in the comments to that gist, but since it relies on

resolve
, it will not work on Clojurescript.

I think

s/form
is behaving correctly in this case, but it would be useful to have an additional function that, given a multi-spec, could return a function or data that could be used to explore the current implementations.






[CLJ-1305] Add optional not-found argument when invoking vectors or sets as functions Created: 12/Dec/13  Updated: 13/Dec/17

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

Type: Feature Priority: Minor
Reporter: Dave Tenny Assignee: Unassigned
Resolution: Unresolved Votes: 11
Labels: None

Attachments: Text File 0001-Added-tests-for-using-get-on-sets-refs-1305.patch     Text File 0001-add-not-found-to-sets-and-vecs-as-functions-refs-130.patch    
Approval: Triaged

 Description   

Maps, keywords, and symbols when used as operators allow optional second arguments for 'default-not-found' values is if to 'get'.

({:a 1} :b 'b) => b

However sets don't support this behavior (though they do with 'get') and vectors don't allow the optional default-not-found in their pseudo 'nth' semantics.

user=> (#{:a  :b} :b 'notfound)
ArityException Wrong number of args (2) passed to: PersistentHashSet  clojure.lang.AFn.throwArity (AFn.java:437)


 Comments   
Comment by Pepijn de Vos [ 12/Nov/14 1:31 PM ]

I fixed the problem with Dirk at the Amsterdam Clojurians Hackathon.

Comment by Bozhidar Batsov [ 12/Nov/14 3:15 PM ]

Guess you can add a couple of unit tests as well.

Comment by Dirk Geurs [ 30/Dec/14 9:51 AM ]

I have added some tests for the changes made earlier.

Comment by Aaron Brooks [ 13/Dec/17 5:58 PM ]

Is there something that is needed for these patches to be applied? I run into this semi-regularly.

Comment by Alex Miller [ 13/Dec/17 9:40 PM ]

Sorry, has just never been looked at afaik. Votes help raise visibility.





[CLJ-2289] conj docstring and arglists don't mention 0- and 1-arity versions Created: 13/Dec/17  Updated: 13/Dec/17

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

Type: Defect Priority: Trivial
Reporter: jcr Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, docstring, documentation

Attachments: Text File CLJ-2298.patch    

 Description   

See: https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/clj/clojure/core.clj#L76

Arglists should be '([] [coll] [coll x] [coll x & xs])

Docstring should mention "(conj coll) returns coll" and "(conj) returns an empty vector", and also the fact that "hence it can be used as a reducing function", clarifying the reason to use [] as the default.






[CLJ-2290] into docstring doesn't mention 0- and 1-arity versions Created: 13/Dec/17  Updated: 13/Dec/17

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

Type: Defect Priority: Trivial
Reporter: jcr Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docs, docstring, documentation

Attachments: Text File CLJ-2290.patch    

 Description   

See: https://github.com/clojure/clojure/blob/clojure-1.9.0-alpha14/src/clj/clojure/core.clj#L6763

Docstring should mention that "(into coll) returns coll" and "(into) returns an empty vector" and the fact that "hence it can be used as a reducing function", clarifying the reason to use [] as the default (the same as with `conj`).






[CLJ-2291] <! and >! break clojure.test/is Created: 13/Dec/17  Updated: 13/Dec/17

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

Type: Defect Priority: Minor
Reporter: jcr Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: clojure.test


 Description   

`(is (not (<! ...)))` works, but `(is (<! ...))` doesn't.

Under the hood, `is` rewrites the code in such a way that it calls `(apply <!
...)`, because `is` assumes that <! is a plain function (see: `assert-expr
:default`[1], `assert-predicate`[2] in clojure.test). It triggers the ">! used
not in (go ...) block" assertion.

[1]https://github.com/clojure/clojure/blob/2e0c0a9a89ede8221504edeb90e8c4ee6cce7e16/src/clj/clojure/test.clj#L486
[2]https://github.com/clojure/clojure/blob/2e0c0a9a89ede8221504edeb90e8c4ee6cce7e16/src/clj/clojure/test.clj#L435

Example code: https://gist.github.com/augustl/4a679dc95847db4434d0e7348651224f#file-test-cljs-L34
Macroexpansion: https://gist.github.com/amalloy/26ec8b8910c7c00bd7feaeef2307bc92#file-gistfile1-txt-L48

Workaround: make `assert-expr` aware of special core.async functions, and use
`assert-any` for those. It's reasonable since core.async is a widely-used core
library. It'd fix the particular problem described in this issue.

The correct solution would be to make <! and >! macros instead of functions.






[CLJ-2288] Add :into parameter to s/keys for spec Created: 12/Dec/17  Updated: 13/Dec/17  Resolved: 13/Dec/17

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

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


 Description   

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

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

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



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

Not interested, thanks.





[CLJ-1255] Support Abstract Base Classes with Java-only variant of "reify" Created: 06/Sep/13  Updated: 12/Dec/17

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

Type: Feature Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 13
Labels: interop

Attachments: Text File 0001-CLJ-1225-add-defclass-extend-class.patch     Text File 0001-CLJ-1225-add-defclass-extend-class-v2.patch    
Approval: Triaged

 Description   

Problem:

  • Various Java APIs depend on extension of abstract base classes rather than interfaces
  • "proxy" has limitations (no access to protected fields or super)
  • "proxy" has performance overhead because of an extra layer of functions / parameter boxing etc.
  • "gen-class" is complex and is complected with compilation / bytecode generation

In summary: Clojure does not currently have a good / convenient way to extend a Java abstract base class dynamically.

The proposal is to create a variant of "reify" that allows the extension of a single abstract base class (optionally also with interfaces/protocols). Code generation would occur as if the abstract base class had been directly extended in Java (i.e. with full access to protected members and with fully type-hinted fields).

Since this is a JVM-only construct, it should not affect the portable extension methods in Clojure (deftype etc.). We propose that it is placed in an separate namespace that could become the home for other JVM-specific interop functionality, e.g. "clojure.java.interop"

Proposed solution: The attached patch proposes an implementation for this feature, providing a new `clojure.interop` namespace, containing the `defclass` and `extend-class` macros.

"(defclass name [fields*] options* super-class super-args specs*)

   Like clojure.core/deftype but can extend a concrete class, override
   and invoke public and protected methods defined in the super class or
   one of its base classes and access/set! public and protected fields of
   those.

   super-args is a (possibly empty) vector of arguments to the superclass
   constructor

   It is possible to invoke a super method while overriding it, by type-hinting
   the parameter `this` as the super class: (. ^SuperClass this method args*)"
"(extend-class options* super-class super-args specs*)

   Like clojure.core/reify but can extend a concrete class, override
   and invoke public and protected methods defined in the super class or
   one of its base classes and access/set! public and protected fields of
   those.

   super-args is a (possibly empty) vector of arguments to the superclass
   constructor

   It is possible to invoke a super method while overriding it, by type-hinting
   the parameter `this` as the super class: (. ^SuperClass this method args*)"

Patch: 0001-CLJ-1225-add-defclass-extend-class-v2.patch



 Comments   
Comment by Alex Miller [ 30/Jun/14 8:18 AM ]

From Rich: we do not want to support abstract classes in a portable construct (reify, deftype). However, this would be considered as a new Java-only construct (extend-class or reify-class). If you could modify the ticket appropriately, will move back to Triaged.

Comment by Nicola Mometto [ 15/May/17 3:19 PM ]

Attached a proposed impl for this feature

Comment by Nicola Mometto [ 15/May/17 3:22 PM ]

More documentation about the proposed impmlementation can be found at https://docs.google.com/document/d/1OcewjSpxmeFRQ3TizcaRRwlV34T8wl4wVED138FHFFE, non squashed commits at https://github.com/clojure/clojure/compare/master...Bronsa:defclass

Comment by Nicola Mometto [ 12/Dec/17 11:29 AM ]

Updated patch is the same as v1 but refreshed on top of master





[CLJ-2286] Update `clj` REPL with hint: (use Crtl-D to exit) Created: 11/Dec/17  Updated: 11/Dec/17

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

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

All


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

 Description   

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

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

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

Patch: clj-2286.patch






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

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

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

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


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

 Description   

Abstract

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

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

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

Related to

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

BENCHMARKS

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

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

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

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

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

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

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

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

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

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

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

INVESTIGATION

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

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

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

THE RESULT WAS: 6,988ms !

SUMMARY

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

A patch attached on this ticket can solve this problem.






[CLJ-2282] Avoid boxing in set! primitive expressions Created: 08/Dec/17  Updated: 08/Dec/17

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler

Attachments: Text File 0001-CLJ-2282-don-t-unnecessarily-box-primitive-set-val.patch    

 Description   

Currently the clojure compiler always emits values to `set!` expression boxed, and unboxes them if necessary. It is possible to enhance AssignExpr to be a MaybePrimitiveExpr and avoid unnecessary box/unboxing.

Assuming this code:

public class Test {
  public int x;
}
(defn foo []
  (let [a (int 1)]
    (set! (.-x (Test.)) a)
    "asd"))

Before:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=1, args_size=0
         0: lconst_1
         1: invokestatic  #17                 // Method clojure/lang/RT.intCast:(J)I
         4: istore_0
         5: new           #19                 // class Test
         8: dup
         9: invokespecial #20                 // Method Test."<init>":()V
        12: checkcast     #19                 // class Test
        15: iload_0
        16: invokestatic  #26                 // Method java/lang/Integer.valueOf:(I)Ljava/lang/Integer;
        19: dup_x1
        20: checkcast     #28                 // class java/lang/Number
        23: invokestatic  #31                 // Method clojure/lang/RT.intCast:(Ljava/lang/Object;)I
        26: putfield      #35                 // Field Test.x:I
        29: pop
        30: ldc           #37                 // String asd
        32: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            5      27     0     a   I
      LineNumberTable:
        line 3: 0
        line 4: 1
        line 5: 19

After:

public static java.lang.Object invokeStatic();
    descriptor: ()Ljava/lang/Object;
    flags: ACC_PUBLIC, ACC_STATIC
    Code:
      stack=3, locals=1, args_size=0
         0: lconst_1
         1: invokestatic  #17                 // Method clojure/lang/RT.intCast:(J)I
         4: istore_0
         5: new           #19                 // class Test
         8: dup
         9: invokespecial #20                 // Method Test."<init>":()V
        12: checkcast     #19                 // class Test
        15: iload_0
        16: dup_x1
        17: putfield      #24                 // Field Test.x:I
        20: pop
        21: ldc           #26                 // String asd
        23: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            5      18     0     a   I
      LineNumberTable:
        line 3: 0
        line 4: 1
        line 5: 16

Patch: 0001-CLJ-2282-don-t-unnecessarily-box-primitive-set-val.patch






[CLJ-2280] Speedup mapv for multiple collections using iterators Created: 03/Dec/17  Updated: 04/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

mapv for multiple collections currently does:

(into [] (map f c1 c2 c3))

I propose to change these to:

(let [it1 (clojure.lang.RT/iter c1)
      it2 (clojure.lang.RT/iter c2)]
     (loop [out (transient [])]
       (if (and (.hasNext it1) (.hasNext it2))
         (recur (conj! out (f (.next it1) (.next it2))))
         (persistent! out))))

This is 5x faster.

For variadic arity we can check if colls is counted?:

  • If yes: Use MultiIterator
  • If no: Forward to map like the current implementation does.

Any thoughts on this or if this could brake something?



 Comments   
Comment by Alex Miller [ 03/Dec/17 10:12 AM ]

Iterators should be treated as dangerous and unsafe by default - they are mutable, stateful, (potentially) non-thread-safe objects. Thus, their use should be carefully scrutinized and in particular you don't want them to a) leak out of a localized context (so they should be eagerly walked and released) or b) be used in a way that triggers the execution of impure functions such that they would be reexecuted later. Sequences cache their realization (and are thread-safe) which makes them slower, but much safer in avoiding this problem.

In this case, you're eagerly producing a concrete collection (not a lazy seq) so the first concern is addressed. However, the second is a little harder to reason through. I think it might be ok, but would have to think about that more.

Note that into uses transduce which uses CollReduce, which will do an iterReduce on Iterable colls, so you could probably get the same effect by simply using the transducer form:

(into [] (map f) c1)

Except that this form only takes a single coll. Multi coll support for transducers exists (see TransformerIterator/createMulti) but is not well-surfaced in the current apis. Seems like leveraging it directly would be vastly preferable to the suggestion above though:

(defn mapv2 [f & cs]
  (let [iters (map #(.iterator %) cs)
        t (clojure.lang.TransformerIterator/createMulti (map f) iters)]
    (loop [out (transient [])]
      (if (.hasNext t)
        (recur (conj! out (.next t)))
        (persistent! out)))))

Note that this assumes all cs are Iterable, which is not a valid assumption. For example: (mapv identity "abc"). So this would still need some work on fallbacks and perf testing.

Comment by A. R [ 04/Dec/17 1:08 AM ]

Alex: I actually did use exactly this very implementation but found it to be quite a bit slower than manually keeping track of the iterators. My guess is it's because of xf.applyTo inside the step function of TransformIterator. We already know source.size() when creating it in createMulti. So this implementation could probably be made smarter by directly invoking the right arity.
Though, I haven't benchmarked anything. So just a guess for now.

Comment by Alex Miller [ 04/Dec/17 8:16 AM ]

Seems like a reasonable guess and something that could be optimized in TransformIterator.

Comment by A. R [ 04/Dec/17 8:34 AM ]

I actually thought about this a bit more. I agree that the proper step here would be to properly leverage transducers and provide better support for multiple collections with transducers.

So I'd suggest changing the game plan to:

1. Make into and transduce work with multiple collections
2. Throw out MultiIterator. We can get much better performance doing this inline in TransformIterator, then we avoid the unnecessary step of "build up sequence in MultiIterator" followed by "spread out this just created sequence in apply".

Though, this would make the TransformIterator much more involved.

After this, we could give many transducers the option of working over multiple sequences (keep, filter, map-indexed, take, drop etc etc) and efficiently using them with into and transduce.

If you agree, I can change the title to reflect the new scope.

Comment by Alex Miller [ 04/Dec/17 9:05 AM ]

You are moving from "enhancing performance of an existing function" into doing api design which has a whole host of larger considerations. I think it's unlikely that we will expand either into or transduce into multi-coll territory and I don't think there are very many transducers that make sense as natively multi-coll. So, I do not think you should expand the scope of this ticket, as I would then decline it.





[CLJ-2281] reduce on primitive arrays doesn't use ArraySeq_TYPE.reduce Created: 03/Dec/17  Updated: 03/Dec/17

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

Type: Enhancement Priority: Minor
Reporter: A. R Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

Currently a reduce on (e.g.) long-array never ends up in the fast ArraySeq_Long.reduce function. Only if the user calls seq on it beforehand.

So:

;; Fast:
(reduce (fn [_ _]) 0 (seq (long-array [1])))
;; >2x slower:
(reduce (fn [_ _]) 0 (long-array [1]))





[CLJ-2169] conj has out-of-date :arglists Created: 27/May/17  Updated: 30/Nov/17

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

Type: Defect Priority: Trivial
Reporter: Michał Marczyk Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-CLJ-2169-add-nullary-unary-overloads-of-conj-to-its-.patch     Text File 0002-CLJ-2169-add-nullary-unary-overloads-of-conj-to-its-.patch    
Patch: Code
Approval: Triaged

 Description   

conj has had nullary and unary overloads since 1.7.0, but its :arglists still only list [coll x] and [coll x & xs].

Prescreened by: Alex Miller



 Comments   
Comment by Michał Marczyk [ 27/May/17 6:05 PM ]

It occurs to me that perhaps the docstring could be updated too to explain (conj).

The new 0002-… patch includes the :arglists change of the 0001-… patch and adds the sentence "(conj) returns []." to the docstring immediately after "(conj nil item) returns (item).".





[CLJ-2279] strange behavior with spec valid? Created: 30/Nov/17  Updated: 30/Nov/17  Resolved: 30/Nov/17

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

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

clojure spec



 Description   

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

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



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

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

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

Closed per request





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

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

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


 Description   

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



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

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





[CLJ-2277] Add fully-qualified specs to explain-data when map fails to contain required keys Created: 28/Nov/17  Updated: 28/Nov/17

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

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

org.clojure/spec.alpha "0.1.143"



 Description   

Consider the following spec

(s/def :example/name string?)
(s/def :example/age pos-int?)
(s/def :wine/age (s/and pos-int?
                        #(< 100)))
(s/def :example/employee (s/keys
                          :req [:example/string]
                          :req-un [:example/age]))

(s/explain-data :example/employee {})
;; #:clojure.spec.alpha{:problems ({:path [], :pred (clojure.core/fn [%] (clojure.core/contains? % :example/string)), :val {}, :via [:example/employee], :in []} {:path [], :pred (clojure.core/fn [%] (clojure.core/contains? % :age)), :val {}, :via [:example/employee], :in []}), :spec :example/employee, :value {}}

The explain-data in this case does give the precise problem: the map is required keys. But the user requires at least two actions to resolve the problem with ":age": either add some fake value for the required keys and look at the new explain-data OR guess which :age spec was intended and look up the spec.

The default "explain" message is fine in this case, but other spec printers may want to give users hints on how they can resolve this problem in one step. This is easy in the case of ":req" because the spec is fully qualified, but not possible in general for ":req-un".

A solution is to include the fully-qualified spec along with the failing predicate in the "problem" data above.






[CLJ-1458] Enhance the performance of map merges Created: 04/Jul/14  Updated: 28/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 8
Labels: performance

Attachments: Text File 0001-very-simple-test-of-the-merge-function.patch     Text File clj-1458-4.patch     Text File CLJ-1458-5.patch     Text File CLJ-1458-6.patch     Text File CLJ-1458-7.patch     Text File CLJ-1458-transient-merge2.patch     Text File CLJ-1458-transient-merge3.patch     Text File CLJ-1458-transient-merge.patch     Text File merge-test-2.patch     File transient-merge.diff    
Patch: Code and Test
Approval: Triaged

 Description   

It would be nice if merge used transients.

Patch

  • clj-1458-7.patch

Approach
Migrate c.c/merge later in core after transients & reduce. Leave older version as merge1 for use in cases the precede the newer definition. Make APersistentMap/conj & ATransientMap/cons aware of IKVReduce.

The attached patch preserves two existing behaviors of merge

  • metadata propagation
  • the right hand side of the merges can be a Map.Entry, an IPersistentVector where size=2, and regular maps.

Screened by:



 Comments   
Comment by Jason Wolfe [ 13/Sep/14 5:09 PM ]

I will take a crack at a patch today.

Comment by Jason Wolfe [ 13/Sep/14 5:42 PM ]

This patch (transient-merge.diff) makes merge, merge-with, and zipmap (since it was right there and could obviously benefit from transients as well) use transients.

Three potential issues:

  • I had to move the functions, since they depend on transient and friends. I assume this is preferable to a forward declaration. This was the best place I could find, but happy to move them elsewhere.
  • I added multiple arities, to avoid potential performance cost of transient-ing a single argument. Happy to undo this if desired.
  • I had to slightly alter the logic in merge-with, since transient maps don't support contains? (or find).
Comment by Michał Marczyk [ 14/Sep/14 12:43 PM ]

I posted a separate ticket for zipmap, with patch, on 30/May/12: CLJ-1005.

Comment by Jason Wolfe [ 14/Sep/14 5:28 PM ]

Ah, sorry if I overstepped then. Happy to remove that change from this patch then if that will simplify things – just let me know.

Comment by Ghadi Shayban [ 28/Dec/14 10:07 PM ]

alternate approach attached delaying merge until after protocols load, and then using transducers.

Comment by Michael Blume [ 28/Dec/14 11:50 PM ]

Looks like you're doing (get m k) twice – shouldn't that be thrown in a local?

Comment by Michael Blume [ 29/Dec/14 1:41 PM ]

um, put, in a local, I mean, 'throw' was a bad choice of word.

Comment by Ghadi Shayban [ 29/Dec/14 2:14 PM ]

Yeah there's that – won't be using get anyways after CLJ-700 gets committed.

We should add performance tests too. merging two maps, three, many maps, also varying the sizes of the maps, and for merge-with, varying the % of collisions.

Need to go back to the (some identity) logic, otherwise metadata is propagated from maps other than the first provided. I'll fix later.

Comment by Michael Blume [ 29/Dec/14 2:49 PM ]

I don't know if this is supposed to be allowed, but this breaks

(merge {} [:foo 'bar])

which is used in the wild by compojure-api

Comment by Michael Blume [ 29/Dec/14 2:49 PM ]

https://github.com/metosin/compojure-api/blob/0.16.6/src/compojure/api/meta.clj#L198

Comment by Michael Blume [ 29/Dec/14 2:54 PM ]

Ghadi, contains? uses get under the covers, so it's still two gets, right? It seems like it'd be more performant to stick with the ::none trick.

Comment by Nicola Mometto [ 29/Dec/14 5:36 PM ]

This calls for if-let + find.

Comment by Ghadi Shayban [ 29/Dec/14 10:37 PM ]

new patch addressing concerns so far

Comment by Ghadi Shayban [ 29/Dec/14 10:48 PM ]

CLJ-1458-transient-merge3.patch removes silly inlining macro, uses singleton fns instead.

Comment by Michael Blume [ 29/Dec/14 11:14 PM ]

Nice =)

This should come with tests. If we want to preserve the ability to merge with a MapEntry, we should test it. This isn't so much a weakness of the patch as of the existing tests. I see merge and merge-with being used a few times in the test suite, but I see no test whose purpose is to test their behavior.

Comment by Michael Blume [ 29/Dec/14 11:17 PM ]

Extremely simple merge test, we need more than this, but this is a start

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

clj-1458-4.patch refreshed to apply to master, no changes.

Comment by Ghadi Shayban [ 09/Jan/16 5:09 PM ]

I'd like to reevaluate the scope of this ticket. Can we address 'merge' only and defer 'merge-with'? It's by far the more common function. I've attached a new simplified patch.

Comment by Ghadi Shayban [ 09/Jan/16 9:50 PM ]

CLJ-1458-6.patch is yet another cleaner approach

Comment by Alex Miller [ 01/Feb/16 5:17 AM ]

Can you update the ticket approach section to discuss the APersistentMap.cons / ASSOC changes. Also, can you add a before / after perf test for one or more common cases?

Comment by Michael Blume [ 28/Sep/16 1:55 PM ]

Updated patch to handle use of merge in core_print before it's defined in core

Comment by Ghadi Shayban [ 28/Sep/16 2:22 PM ]

If anyone wants to take stewardship of this, go ahead. I had trouble getting consistent performance improvements on this. Obviously this needs fresh benchmarks.

Comment by Alex Miller [ 28/Sep/16 2:28 PM ]

Yes, this needs a benchmark showing demonstrable improvement. The whole goal here is improved perf - if we can't prove it's consistently faster, then there is no point in even reviewing it.

Comment by Ghadi Shayban [ 28/Nov/17 4:40 PM ]

This ticket needs help. Step 0 is writing a benchmark harness that exercises maps of various sizes, and the various polymorphic arguments below.

After a benchmark harness, implementation code needs to preserve this behavior:

  • If the first argument is not nil, its metadata propagates to the result.
  • The arguments can be a Map.Entry, an IPersistentVector where size=2, and regular maps.
  • When passed a non-map as the first argument, result is undefined.




[CLJ-2276] Add build for standalone jar Created: 27/Nov/17  Updated: 27/Nov/17  Resolved: 27/Nov/17

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

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

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

 Description   

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

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

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

# Ant:
./antsetup.sh
ant local

# Run:
java -jar clojure.jar





[CLJ-2275] case fails for vectors with negative numbers Created: 27/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Major
Reporter: Max Lorenz Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: case
Environment:

OSX


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

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

The last case should return true like the other,

Real life example that triggered this:

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

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

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



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

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





[CLJ-2269] definterface seems not to resolve imported classes in type hints Created: 16/Nov/17  Updated: 27/Nov/17

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

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

Java 1.8.0_152
Windows 10


Approval: Triaged

 Description   

For example:

user=> (import java.util.Map)
java.util.Map
user=> (definterface Foo (^void foo [^Map map]))
user.Foo
user=> (deftype Bar [] Foo (foo [this m]))
CompilerException java.lang.NoClassDefFoundError: java/lang/Map, compiling:(NO_SOURCE_PATH:3:1)
user=> (definterface Foo2 (^void foo2 [^java.util.Map map]))
user.Foo2
user=> (deftype Bar2 [] Foo2 (foo2 [this m]))
user.Bar2

The attempt to type-hint with just Map fails; you have to use java.util.Map to get a usable interface definition.






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

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

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

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

 Description   

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

Patch: clj-2266.patch



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

Doesn't valid? call conform?

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

fair enough





[CLJ-2271] "caller" information missing in explain-data during macro instrumentation failure Created: 19/Nov/17  Updated: 27/Nov/17

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

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

org.clojure/spec.alpha "0.1.143"


Approval: Triaged

 Description   

When there is a instrumentation failure for a function, the explain-data includes "caller" information. However, this information is missing if the instrumentation failure is for a macro.

This comment has led me to believe that the intended behavior is for explain-data to contain this info, so third-party error printers can display it.

In the repro below, I'm setting up a custom printer just to capture the raw explain-data (it's not a useful printer, just a means to show what is happenening)

Repro:

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


  (s/fdef my-fn
          :args (s/cat :x int?))
  (defn my-fn [x]
    x)

  (s/fdef my-macro
          :args (s/cat :x int?))
  (defmacro my-macro [x]
    x)

  (st/instrument)
  (def !ed (atom nil))
  (set! s/*explain-out* (fn [ed]
                          (reset! !ed ed)))
  (my-fn "")
  @!ed
  ;; {:clojure.spec.alpha/problems [{:path [:args :x], :pred clojure.core/int?, :val "", :via [], :in [0]}], :clojure.spec.alpha/spec #object[clojure.spec.alpha$regex_spec_impl$reify__2436 0x72029b0e "clojure.spec.alpha$regex_spec_impl$reify__2436@72029b0e"], :clojure.spec.alpha/value (""), :clojure.spec.alpha/args (""), :clojure.spec.alpha/failure :instrument, :clojure.spec.test.alpha/caller {:file "form-init8333540581183382896.clj", :line 548, :var-scope expound.alpha/eval27394}}

  ;; ^--- Note there is an entry for :clojure.spec.test.alpha/caller

  (my-macro "")
  @!ed

  ;; #:clojure.spec.alpha{:problems [{:path [:args :x], :pred clojure.core/int?, :val "", :via [], :in [0]}], :spec #object[clojure.spec.alpha$regex_spec_impl$reify__2436 0x479a6a73 "clojure.spec.alpha$regex_spec_impl$reify__2436@479a6a73"], :value (""), :args ("")}

  ;; ^--- No caller information


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

You can't instrument a macro, so that part of the ticket doesn't make sense as written. But I expect you mean the spec check during macro expansion.

In the macro check case, the caller info is known by the compiler and included in the wrapper CompilerException. I suppose that info could be passed into s/macroexpand-check from the Compiler and possibly produce similar results as with instrumented function calls.





[CLJ-2272] Suppress repl printing of ex-data for macro spec errors Created: 20/Nov/17  Updated: 27/Nov/17

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

Type: Enhancement Priority: Major
Reporter: Greg Chapman Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When a macro fails a spec check at the repl, the message printed includes the (often large) ex-data map generated by spec explain:

Clojure 1.9.0-RC1
user=> (let [x] 5)
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/let did not conform to spec:
In: [0] val: () fails spec: :clojure.core.specs.alpha/bindings at: [:args :bindings :init-expr] 
predicate: any?,  Insufficient input
 #:clojure.spec.alpha{:problems [{:path [:args :bindings :init-expr], :reason "Insufficient input", 
:pred clojure.core/any?, :val (), :via [:clojure.core.specs.alpha/bindings 
:clojure.core.specs.alpha/bindings], :in [0]}], :spec 
#object[clojure.spec.alpha$regex_spec_impl$reify__1188 0x3f9270ed 
"clojure.spec.alpha$regex_spec_impl$reify__1188@3f9270ed"], :value ([x] 5), :args ([x] 5)}, 
compiling:(NO_SOURCE_PATH:18:1)

As I understand it, checkSpecs in Compiler.java calls macroexpand-check from spec. For failures, macroexpand-check throws an ExceptionInfo with the explain data in the data map. checkSpecs catches this and throws a new CompilerException, passing the ExceptionInfo as the cause. The CompilerException constructor calls toString on the cause, which for ExceptionInfo generates a string with both the message and the data map.

Possibly, you could add a new overload of the CompilerException constructor, which specifies both an explicit message and a cause (using the message in a call to errorMsg). Then checkSpecs could use this overload, passing ExceptionInfo.getMessage and avoiding printing of the data map.

I've marked this as major, because I think the data map dumps have the potential to be intimidating to newcomers.



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

There are multiple contributors here to the messiness of the resulting message and I don't agree with the suggested solution. CompilerExceptions are designed to be wrappers (with location info) that can be unrolled to find the cause exception, so to some degree the repl catch handler bears some of the blame here - it should be doing more to unwrap and report. And it could even be doing more for specifically for spec errors if we had a marker that identified them. Also, I think the macro check should not be including the explain data string.





[CLJ-2273] Add original 'assert' form to explain-data for s/assert Created: 21/Nov/17  Updated: 27/Nov/17

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

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

org.clojure/spec.alpha "0.1.143"


Approval: Triaged

 Description   

Using s/assert instead of assert has advantages: namely a specific reason why the data failed to meet the spec. But it also has disadvantages: the error message does not contain the assertion code the user wrote, so it's harder to see which assertion failed.

I believe we could have the best of both world if s/assert contained the original "assert" code - that way the error message could (optionally) print out this information.

(require '[clojure.spec.alpha :as s])
  (s/check-asserts true)

  (let [x 1]
    (s/assert string? x))

  ;; Spec assertion failed val: 1 fails predicate:
  ;; :clojure.spec.alpha/unknown

  (let [x 1]
    (assert (string? x)))

  ;; Assert failed: (string? x)





[CLJ-2274] Line numbers in stack trace are wrong when type hints satisfaction fails Created: 26/Nov/17  Updated: 27/Nov/17

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

Type: Defect Priority: Minor
Reporter: Stijn Seghers Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

I only tested this on macOS 10.13.1 with both Clojure 1.8.0 and 1.9.0-RC1.


Approval: Triaged

 Description   

When I run the following file

example.clj
(defn f [^double x] x)

(defn g []
  (println)
  (f nil))

(g)

I get the following stack trace

Exception in thread "main" java.lang.NullPointerException, compiling:(/.../example.clj:7:1)
	at clojure.lang.Compiler.load(Compiler.java:7526)
	at clojure.lang.Compiler.loadFile(Compiler.java:7452)
	at clojure.main$load_script.invokeStatic(main.clj:278)
	at clojure.main$script_opt.invokeStatic(main.clj:338)
	at clojure.main$script_opt.invoke(main.clj:333)
	at clojure.main$main.invokeStatic(main.clj:424)
	at clojure.main$main.doInvoke(main.clj:387)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.lang.Var.applyTo(Var.java:702)
	at clojure.main.main(main.java:37)
Caused by: java.lang.NullPointerException
	at clojure.lang.RT.doubleCast(RT.java:1348)
	at user$g.invokeStatic(example.clj:4)
	at user$g.invoke(example.clj:3)
	at user$eval147.invokeStatic(example.clj:7)
	at user$eval147.invoke(example.clj:7)
	at clojure.lang.Compiler.eval(Compiler.java:7062)
	at clojure.lang.Compiler.load(Compiler.java:7514)
	... 9 more

The line at user$g.invokeStatic(example.clj:4) here is quite misleading, because the error happens at line 5.






Generated at Sun Jan 21 12:37:25 CST 2018 using JIRA 4.4#649-r158309.