<< Back to previous view

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

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

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

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



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

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


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

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

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

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





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

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

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

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



 Description   

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

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

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

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



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

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





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

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

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

Tested against clojure 1.7 and 1.8



 Description   

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

Steps to reproduce:

File macro.clj:

(ns macro)

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

REPL:

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

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

PS. a workaround is to define the macro like

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


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

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





[CLJ-840] Add a way to access the current test var in :each fixtures for clojure.test Created: 21/Sep/11  Updated: 23/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: clojure.test

Attachments: File add-test-var.diff     File clj840-20161122.diff     File clj840-2.diff    
Patch: Code
Approval: Triaged

 Description   

When looking at (log) output from tests written with clojure.test, I would like to be able to identify the output associated with each test. A mechanism to expose the current test var within an :each fixture would enable this.

One mechanism might be to bind a test-var var with the current test var before calling the each-fixture-fn in clojure.test/test-all-vars.

Proposed: The proposed patch (clj840-20161122.diff) allows 'each' fixtures to access the var associated with the currently executing test by using (first *testing-vars*). As a result of this change, 'each' fixtures are able to access the metadata associated with the current test var, including the name.

The patch achieves the above by changing the order in which functions are wrapped when a test and its associated 'each' fixtures are run. Before this patch, 'each' fixtures were combined into a single higher-order function, which was then given a thunk containing an invocation of the test-var function to execute as its body. After this patch, the test-var function is now responsible for joining and executing 'each' fixtures but, importantly, it does so within the scope of the binding expression that adds the current test var to *testing-vars*. test-var now invokes the joined fixtures function, rather than the joined fixtures function being given a thunk that invokes test-var.

Patch: clj840-20161122.diff



 Comments   
Comment by Stuart Sierra [ 07/Oct/11 4:33 PM ]

Or just pass the Var directly into the fixture. Vars are invokable.

Comment by Hugo Duncan [ 07/Oct/11 4:45 PM ]

I don't think that works, since the the function passed to the fixture is not the test var, but a function calling test-var on the test var.

Comment by Hugo Duncan [ 21/Oct/11 10:34 PM ]

Patch to add test-var

Comment by Stuart Sierra [ 25/Oct/11 6:04 PM ]

*testing-vars* already has this information, but it's not visible to the fixture functions because it gets bound inside test-var.

Perhaps the :each fixture functions should be called in test-var rather than in test-all-vars. (The namespace of a Var is available in its metadata.) But then we have to call join-fixtures inside test-var every time.

Comment by Stuart Sierra [ 25/Oct/11 6:26 PM ]

Try this patch: clj840-2.diff.

This makes *testing-vars* visible to :each fixture functions, which seems intuitively more correct.

BUT it slightly changes the behavior of test-var, which I'm less happy about.

Comment by Hugo Duncan [ 25/Oct/11 8:07 PM ]

Might it make sense to provide a function on top of testing-vars to return the current test-var?

Comment by Stuart Sierra [ 28/Oct/11 9:14 AM ]

No, that function is first

Comment by Hugo Duncan [ 28/Oct/11 11:31 AM ]

I agree with having the dynamic vars as part of the extension interface, but would have thought that having a function for use when writing tests would have been cleaner. Just my 2c.

Comment by Andy Fingerhut [ 23/Nov/13 12:42 AM ]

With a commit made on Nov 22, 2013, patch clj840-2.diff no longer applies cleanly to latest master. Updating it appears like it might be straightforward, but best for someone who knows this part of the code well to do so.

Comment by Joe Littlejohn [ 22/Nov/16 11:30 AM ]

I'd find it very useful if this one was fixed.

I've added an updated patch that has the same content as clj840-2.diff but applies against the current master (c0326d2), as of November 22nd 2016.

Comment by Joe Littlejohn [ 28/Nov/16 5:59 AM ]

I realise I've only translated a patch provided by someone else here, but if there's anything further you think this one needs before it's in a fit state to be considered then please do shout and I'll endeavour to add something further. Thanks.

Comment by Alex Miller [ 29/Nov/16 8:56 AM ]

If you could update the ticket to better describe the approach of the patch that would be helpful.

Comment by Joe Littlejohn [ 02/Dec/16 9:08 AM ]

The proposed patch (clj840-20161122.diff) allows 'each' fixtures to access the var associated with the currently executing test by using (first *testing-vars*). As a result of this change, 'each' fixtures are able to access the metadata associated with the current test var, including the name.

The patch achieves the above by changing the order in which functions are wrapped when a test and its associated 'each' fixtures are run. Before this patch, 'each' fixtures were combined into a single higher-order function, which was then given a thunk containing an invocation of the test-var function to execute as its body. After this patch, the test-var function is now responsible for joining and executing 'each' fixtures but, importantly, it does so within the scope of the binding expression that adds the current test var to *testing-vars*. test-var now invokes the joined fixtures function, rather than the joined fixtures function being given a thunk that invokes test-var.

Hopefully that's clear, ish

Comment by Joe Littlejohn [ 23/Mar/17 11:42 AM ]

Any chance this one could be considered for inclusion now? The file clj840-20161122.diff is still valid and can be applied to the current HEAD of master.

Comment by Alex Miller [ 23/Mar/17 2:56 PM ]

Can you check that changing the function ordering here hasn't affected the stack trace inspection in do-report? Basically it would be helpful to know in all the various cases (neither each nor once, each but not once, once but not each, and both) that error reporting is unaffected for all those cases. Also, tools like Leiningen monkeypatch parts of this as well. I think all of that's fine, but I'm probably not going to get around to checking all that soon.





[CLJ-2135] clojure.spec/Spec implementations that don't implement IObj get silently dropped in s/def Created: 23/Mar/17  Updated: 23/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec

Attachments: Text File clj-2135.patch    
Patch: Code
Approval: Triaged

 Description   
(deftype MySpec []
  s/Spec
  (conform* [_ x]
    ::s/invalid))

(s/def ::x (MySpec.))

(s/explain ::x :foo)

This will fail with a "Unable to resolve spec: :user/x" exception, but the def succeeded. Switching the deftype to defrecord fixes the problem.

Cause: The with-name function has cond options for ident?, regex?, and IObj. If none of these succeed, there is no fallthrough case and the s/def will silently return nil.

Proposed: Throw an error in the fallthrough case.

Patch: clj-2135.patch






[CLJ-2134] Update the docstring of `with-redefs` to reflect how little the macro should be used Created: 23/Mar/17  Updated: 23/Mar/17

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

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

Approval: Triaged

 Description   

Currently the docstring for `with-redefs` recommends itself for use in testing. However there are a number of reasons why using this macro for testing is suboptimal:

  • `with-redefs` "bindings" are not transferred to new threads since it's a global mutation
  • users can get runtime errors if they redef a primitive type-hinted function to a function taking only objects
  • If parts of the body of `with-redefs` is delayed (via a delay, go block, etc.) that code may not see the new root
  • The mutation is global so it "leaks" outside the current scope into other code that may currently be running in another thread
  • Clojure tends to shun global mutation, and yet this macro isn't marked with a `!` nor properly warns users about the dangers mentioned here

Due to these reasons I often encounter new users using `with-redefs` without understanding the ramifications of doing so. All this behavior makes sense if a user understands how Vars work, but that's a lot of knowledge to take on for a new user.

Suggestion:
Remove the suggestion that `with-redefs` be used in testing
Add a few notes of warning about global mutation, and concurrency issues with the macro.






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

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

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

N/A



 Description   

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






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

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

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

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

 Description   

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

mvn clean install

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






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

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

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


 Description   

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

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

e.g

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

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

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



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

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

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

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

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

closed per request





[CLJ-1381] Improve support for extending protocols to primitive arrays Created: 13/Mar/14  Updated: 18/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: protocols

Approval: Vetted

 Description   

It is possible to extend protocols to primitive arrays but specifying the class for the type is a little tricky:

(defprotocol P (p [_]))
(extend-protocol P (Class/forName "[B") (p [_] "bytes"))
(p (byte-array 0))   ;; => "bytes"

However, things go bad if you try to do more than one of these:

(extend-protocol P 
  (Class/forName "[B") (p [_] "bytes")
  (Class/forName "[I") (p [_] "ints"))
CompilerException java.lang.UnsupportedOperationException: nth not supported on this type: Character, compiling:(NO_SOURCE_PATH:1:1)
	clojure.lang.Compiler.analyze (Compiler.java:6380)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$MapExpr.parse (Compiler.java:2879)
	clojure.lang.Compiler.analyze (Compiler.java:6369)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$InvokeExpr.parse (Compiler.java:3624)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6562)
	clojure.lang.Compiler.analyze (Compiler.java:6361)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$BodyExpr$Parser.parse (Compiler.java:5708)
	clojure.lang.Compiler$FnMethod.parse (Compiler.java:5139)
	clojure.lang.Compiler$FnExpr.parse (Compiler.java:3751)
Caused by:
UnsupportedOperationException nth not supported on this type: Character
	clojure.lang.RT.nthFrom (RT.java:857)
	clojure.lang.RT.nth (RT.java:807)
	clojure.core/emit-hinted-impl/hint--5951/fn--5953 (core_deftype.clj:758)
	clojure.core/map/fn--4207 (core.clj:2487)
	clojure.lang.LazySeq.sval (LazySeq.java:42)
	clojure.lang.LazySeq.seq (LazySeq.java:60)
	clojure.lang.RT.seq (RT.java:484)
	clojure.lang.RT.countFrom (RT.java:537)
	clojure.lang.RT.count (RT.java:530)
	clojure.lang.Cons.count (Cons.java:49)
	clojure.lang.Compiler.analyze (Compiler.java:6352)
	clojure.lang.Compiler.analyze (Compiler.java:6322)

The code in {parse-impls} is seeing the second {(Class/forName "[I")} as a function, not as a new type. One workaround for this is to only extend the protocol to one type at a time.

It would be even better (moving into enhancement area) if there was a syntax here to specify primitive array types - we already have the syntax of {bytes, ints, longs}, etc for type hints and those seem perfectly good to me.



 Comments   
Comment by Nahuel Greco [ 18/Sep/14 6:08 PM ]

It also breaks when extending only one array type:

(extend-protocol P
  String               (p [_] "string")
  (Class/forName "[B") (p [_] "ints") 
  )

;=> CompilerException java.lang.UnsupportedOperationException: nth not supported on this type ...

But changing the declaration order fixes it:

(extend-protocol P
  (Class/forName "[B") (p [_] "ints") 
  String               (p [_] "string")
  )

;=> OK
Comment by Alex Miller [ 12/Jan/16 3:16 PM ]

Dupe of CLJ-1790

Comment by Alex Miller [ 07/Jun/16 4:00 PM ]

On further inspection, I don't think this is a dupe of CLJ-1790 but merely a related problem.

Comment by Greg Chapman [ 18/Mar/17 11:04 AM ]

Using Class/forName has a further problem, as type-hints on the this parameter are longer emitted:

user=> (extend-protocol P (Class/forName "[B") (p [this] (aget this 0)))
Reflection warning, NO_SOURCE_PATH:2:50 - call to static method aget on clojure.lang.RT can't be resolved (argument types: unknown, int).

Reflection warnings are also generated for non-primitive arrays (so just supporting ints etc, won't completely fix this problem). It would be good to have a solution which covered all the problems with extending protocols to arrays.





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

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

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


 Description   

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

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

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

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



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

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

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

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

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

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

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

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





[CLJ-2129] Enhance CompilerException to optionally return the invalid form Created: 16/Mar/17  Updated: 16/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: compiler, error-reporting

Approval: Triaged

 Description   

Currently CompilerException wraps errors that occur at compile time and adds file/line/col info. Some tools could do more with the failing form, particularly if it includes useful meta.

Proposal is to add a CompilerException constructor that also takes the form (Object) and conveys it. Existing uses like CLI and REPL would do nothing different, but a tooling user of the Compiler could use that information.

Possible issue: if the form is lazy or large?



 Comments   
Comment by Thomas Heller [ 16/Mar/17 12:29 PM ]

I added the latest form to the CompilerException when available but that form contains basically no metadata.

user=> (defn x
  :foo)
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/defn did not conform to spec:
...

user=> (binding [*print-meta* true] (prn (.-form *e)))
^{:line 17, :column 1} (defn x :foo)

That information is already present in the CompilerException. Having the form provides minimal benefit in this case since it has lost all other source information and we cannot tell how this look in the the source. To get a source mapping for tools so they can highlight the correct area it would still need to read the form itself (and probably not using the form from the Exception).

Comment by Thomas Heller [ 16/Mar/17 12:37 PM ]

Looking into LINE_BEFORE, COLUMN_BEFORE, LINE_AFTER, COLUMN_AFTER now to potentially provide the exact boundaries of the form if possible.

Comment by Thomas Heller [ 16/Mar/17 2:00 PM ]

I added the current reader location to the Compiler Exception [1].

user=> (load-file "/Users/zilence/code/shadow-devtools/src/dev/demo/defn_error.clj")
CompilerException clojure.lang.ExceptionInfo: Call to clojure.core/defn did not conform to spec:
...
user=> (.. *e -location -lineBefore)
4
user=> (.. *e -location -lineAfter)
6
;; column is 1 before/after, source is
(defn x
  :foo)

This would at least allow extracting the entire source string of the form easily. When using a reader however it could start at the location already provided by the CompilerException and it would find the end on its own. The bindings required for the reader location are also lost when working in a REPL, so it always points to 0/0-0/0. Not sure this is worth pursuing.

[1] https://github.com/clojure/clojure/compare/master...thheller:CLJ-2128





[CLJ-2128] spec error during macroexpand no longer throws compiler exception with location Created: 16/Mar/17  Updated: 16/Mar/17

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: error-reporting, regression, spec
Environment:

1.9.0-alpha12-1.9.0-alpha15


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

 Description   

This used to work but got out of sync with the commit https://github.com/clojure/clojure/commit/b3d3a5d6ff0a2f435bb6a5326da2b960038adad4, which changed the IllegalArgumentException to an ex-info, but didn't change the corresponding catch in the Compiler. That change is visible as of 1.9.0-alpha12.

Before alpha12:

...
Exception in thread "main" java.lang.IllegalArgumentException: Call to clojure.core/ns did not conform to spec:
In: [1] val: ((:required clojure.string)) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
:clojure.spec/args  (th.core (:required clojure.string))
, compiling:(th/core.clj:1:1)
...

^^ note the "th/core.clj:1:1"

After alpha12:

...
Exception in thread "main" clojure.lang.ExceptionInfo: Call to clojure.core/ns did not conform to spec:
In: [1] val: ((:required clojure.string)) fails at: [:args] predicate: (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses),  Extra input
:clojure.spec/args  (th.core (:required clojure.string))
 #:clojure.spec{:problems [{:path [:args], :reason "Extra input", :pred (cat :docstring (? string?) :attr-map (? map?) :clauses :clojure.core.specs/ns-clauses), :val ((:required clojure.string)), :via [], :in [1]}], :args (th.core (:required clojure.string))}, compiling:(/private/var/folders/7r/_1fj0f517rgcxwx79mn79mfc0000gn/T/form-init4120559363887828149.clj:1:125)
	at clojure.lang.Compiler.load(Compiler.java:7441)
...

Patch: clj-2128.patch






[CLJ-2066] Reflection on internal classes fails under Java 9 Created: 22/Nov/16  Updated: 16/Mar/17

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

Type: Defect Priority: Major
Reporter: Toby Crawley Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: interop, reflection, regression

Attachments: Text File tcrawley.CLJ-2066.2017-03-16.patch    
Patch: Code and Test
Approval: Incomplete

 Description   

Due to changes in reflective access for the Jigsaw module system in Java 9, the Reflector will now fail on some cases that worked in previous Java versions.

(def fac (javax.xml.stream.XMLInputFactory/newInstance))
(.createXMLStreamReader fac (java.io.StringReader. ""))

Here fac will be an instance of com.sun.xml.internal.stream.XMLInputFactoryImpl, which is an implementation of javax.xml.stream.XMLInputFactory. In the new java.xml module, javax.xml.stream is an exported package, but the XMLInputFactoryImpl is an internal implementation of the public interface in that package. The invocation of createXMLStreamReader will be reflective and the Reflector will attempt to invoke the method based on the implementation class, which is not accessible outside the module, yielding:

IllegalAccessException class clojure.lang.Reflector cannot access class com.sun.xml.internal.stream.XMLInputFactoryImpl (in module java.xml) because module java.xml does not export com.sun.xml.internal.stream to unnamed module @4722ef0c
	jdk.internal.reflect.Reflection.throwIllegalAccessException (Reflection.java:423)
	jdk.internal.reflect.Reflection.throwIllegalAccessException (Reflection.java:414)
	jdk.internal.reflect.Reflection.ensureMemberAccess (Reflection.java:112)
	java.lang.reflect.AccessibleObject.slowCheckMemberAccess (AccessibleObject.java:632)
	java.lang.reflect.AccessibleObject.checkAccess (AccessibleObject.java:624)
	java.lang.reflect.Method.invoke (Method.java:539)
	clojure.lang.Reflector.invokeMatchingMethod (Reflector.java:93)
	clojure.lang.Reflector.invokeInstanceMethod (Reflector.java:28)

One workaround here is to avoid the reflective call by type-hinting to the public exported interface:

(.createXMLStreamReader ^javax.xml.stream.XMLInputFactory fac (java.io.StringReader. ""))

Another (undesirable) workaround is to export the private package from java.xml to the unnamed module (which is the module used when code is loaded from the classpath rather than from a module) when invoking java/javac:

java --add-exports=java.xml/com.sun.xml.internal.stream=ALL-UNNAMED --add-exports=java.xml/com.sun.xml.internal.stream.writers=ALL-UNNAMED --add-exports=java.xml/com.sun.org.apache.xerces.internal.impl=ALL-UNNAMED ...

Proposed: The attached patch will check for and catch IllegalAccessException in the Reflector. When it occurs, the Reflector will attempt to invoke the method on all super-classes and super-interfaces in case one of them can succeed.

Patch: tcrawley.CLJ-2066.2017-02-14.patch

More info:



 Comments   
Comment by Toby Crawley [ 22/Nov/16 10:02 AM ]

This is the root cause of http://dev.clojure.org/jira/browse/DXML-32

Comment by Toby Crawley [ 14/Feb/17 3:48 PM ]

I don't see any indication that Java 9 will change in our favor, so I'm attaching a patch to fix this in Clojure itself.

Comment by Alex Miller [ 07/Mar/17 4:04 PM ]

Screening comments:

I think by just inspecting the patch that the new code will attempt to invoke the method on all super classes and interfaces, many of which will not have the applicable method and will thus throw IllegalArgumentException, possibly before finding the correct one. Can we reduce the effort here by only attempting the call on the super types that actually have the method? It would be good to expand the tests to also include this case if possible if I'm just mis-reading things (I did not attempt to construct this scenario).

Also, is there a method we can call to determine whether the method is accessible before invoking and needing to catch IllegalAccessException? (Obviously, we would want something that is not new in Java 9 so that this call worked on older JDKs too.)

Comment by Herwig Hochleitner [ 07/Mar/17 5:37 PM ]

This may be a stupid question, but has there been any research, whether [1] and [2] could be utilized to handle this without catching exceptions?

[1] http://download.java.net/java/jdk9/docs/api/java/lang/Class.html#getModule--
[2] http://download.java.net/java/jdk9/docs/api/java/lang/reflect/Module.html#isExported-java.lang.String-java.lang.reflect.Module-

Comment by Alex Miller [ 07/Mar/17 6:27 PM ]

Nope, that's the kind of thing I was asking about in my comment. However, those are jdk 9 only methods and right now Clojure supports jdk 6+. That's not impossible to deal with but does increase the difficulty.

Comment by Toby Crawley [ 08/Mar/17 2:30 PM ]

Alex: you are correct, an intermediate ancestor that does not provide the method will cause the lookup to fail - I'll rework and provide a new patch with better tests.

Herwig: we could build Clojure as a multi-release jar in order to have code that can use Java 9 features, but that would definitely complicate the build. However, there may be other Java 9 issues that may require us to take that option (I'm investigating one potential issue now).

Comment by Alex Miller [ 08/Mar/17 3:28 PM ]

Yeah, I was looking at the multi release jar stuff yesterday. I would really like to avoid that if at all possible as it adds a bunch of work for build, ci, and test.

Comment by Toby Crawley [ 13/Mar/17 9:42 PM ]

I've attached a new patch (tcrawley.CLJ-2066.2017-03-13.patch) and removed the old one (tcrawley.CLJ-2066.2017-02-14.patch). The new patch checks for any matching methods before calling invokeMatchingMethod; this will prevent the lack of the method on an ancestor from throwing IllegalArgumentException and aborting the lookup process.

I wasn't able to provide a test for the case where the method was missing on an ancestor, since that requires trying to invoke the method through an interface that doesn't provide it (all subclasses will provide methods from parent classes, so you need an ancestor tree that pulls in an interface), and I couldn't find anything in Java's stdlib that did that and was a non-exposed class. To implement a proper test for this, I believe we'd need to construct a module ourselves, which would require changes to the build process to build that module (and run the test) only under Java 9. I did, however, confirm locally that the old code was broken in this regard, and that the new patch fixes it.

Comment by Toby Crawley [ 14/Mar/17 12:00 PM ]

After reviewing tcrawley.CLJ-2066.2017-03-13.patch, I can see a new issue with it: if the method being invoked exists only on the non-public class, then the patch will throw an IllegalArgumentException claiming that the method doesn't exist instead of the original, more accurate IllegalAccessException. I'll rework and provide another patch.

Comment by Toby Crawley [ 16/Mar/17 9:22 AM ]

New patch attached (tcrawley.CLJ-2066.2017-03-16.patch) that will properly throw the original IllegalAccessException when the method only exists on the non-public class. Patch tcrawley.CLJ-2066.2017-03-13.patch has been deleted.





[CLJ-2127] clojure.spec/def requires literal keyword as first argument Created: 15/Mar/17  Updated: 15/Mar/17  Resolved: 15/Mar/17

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

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


 Description   

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

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

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



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

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

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

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

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

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

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

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

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

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

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





[CLJ-1544] AOT bug involving namespaces loaded before AOT compilation started Created: 01/Oct/14  Updated: 14/Mar/17

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

Type: Defect Priority: Critical
Reporter: Allen Rohner Assignee: Unassigned
Resolution: Unresolved Votes: 15
Labels: aot

Attachments: Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co.patch     Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v2.patch     Text File 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch     Text File 0001-CLJ-1641-disallow-circular-dependencies-even-if-the-.patch    
Patch: Code
Approval: Incomplete

 Description   

If namespace "a" that is being AOT compiled requires a namespace "b" that has been loaded but not AOT compiled, the classfile for that namespace will never be emitted on disk, causing errors when compiling uberjars or in other cases.

A minimal reproducible case is described in the following comment: http://dev.clojure.org/jira/browse/CLJ-1544?focusedCommentId=36734&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-36734

Other examples of the bug:
https://github.com/arohner/clj-aot-repro
https://github.com/methylene/class-not-found

A real issue triggered by this bug: https://github.com/cemerick/austin/issues/23

Related ticket: CLJ-1641 contains descriptions and comments about some potentially unwanted consequences of applying proposed patch 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch

Approach: The approach taken by the attached patch is to force reloading of namespaces during AOT compilation if no matching classfile is found in the compile-path or in the classpath

Patch: 0001-CLJ-1544-force-reloading-of-namespaces-during-AOT-co-v3.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 04/Dec/14 12:45 PM ]

Possibly related: CLJ-1457

Comment by Nicola Mometto [ 05/Dec/14 4:51 AM ]

Has anyone been able to reproduce this bug from a bare clojure repl? I have been trying to take lein out of the equation for an hour but I don't seem to be able to reproduce it – this makes me think that it's possible that this is a lein/classlojure/nrepl issue rather than a compiler/classloader bug

Comment by Nicola Mometto [ 06/Dec/14 4:20 PM ]

I was actually able to reproduce and understand this bug thanks to a minimal example reduced from a testcase for CLJ-1413.

>cat error.sh
#!/bin/sh

rm -rf target && mkdir target

java -cp src:clojure.jar clojure.main - <<EOF
(require 'myrecord)
(set! *compile-path* "target")
(compile 'core)
EOF

java -cp target:clojure.jar clojure.main -e "(use 'core)"

> cat src/core.clj
(in-ns 'core)
(clojure.core/require 'myrecord)
(clojure.core/import myrecord.somerecord)

>cat src/myrecord.clj
(in-ns 'myrecord)
(clojure.core/defrecord somerecord [])

> ./error.sh
Exception in thread "main" java.lang.ExceptionInInitializerError
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:344)
	at clojure.lang.RT.classForName(RT.java:2113)
	at clojure.lang.RT.classForName(RT.java:2122)
	at clojure.lang.RT.loadClassForName(RT.java:2141)
	at clojure.lang.RT.load(RT.java:430)
	at clojure.lang.RT.load(RT.java:411)
	at clojure.core$load$fn__5403.invoke(core.clj:5808)
	at clojure.core$load.doInvoke(core.clj:5807)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5613)
	at clojure.core$load_lib$fn__5352.invoke(core.clj:5653)
	at clojure.core$load_lib.doInvoke(core.clj:5652)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$load_libs.doInvoke(core.clj:5691)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:630)
	at clojure.core$use.doInvoke(core.clj:5785)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at user$eval212.invoke(NO_SOURCE_FILE:1)
	at clojure.lang.Compiler.eval(Compiler.java:6767)
	at clojure.lang.Compiler.eval(Compiler.java:6730)
	at clojure.core$eval.invoke(core.clj:3076)
	at clojure.main$eval_opt.invoke(main.clj:288)
	at clojure.main$initialize.invoke(main.clj:307)
	at clojure.main$null_opt.invoke(main.clj:342)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:383)
	at clojure.lang.AFn.applyToHelper(AFn.java:156)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.io.FileNotFoundException: Could not locate myrecord__init.class or myrecord.clj on classpath.
	at clojure.lang.RT.load(RT.java:443)
	at clojure.lang.RT.load(RT.java:411)
	at clojure.core$load$fn__5403.invoke(core.clj:5808)
	at clojure.core$load.doInvoke(core.clj:5807)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5613)
	at clojure.core$load_lib$fn__5352.invoke(core.clj:5653)
	at clojure.core$load_lib.doInvoke(core.clj:5652)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$load_libs.doInvoke(core.clj:5691)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:628)
	at clojure.core$require.doInvoke(core.clj:5774)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at core__init.load(Unknown Source)
	at core__init.<clinit>(Unknown Source)
	... 33 more

This bug also has also affected Austin: https://github.com/cemerick/austin/issues/23

Essentially this bug manifests itself when a namespace defining a protocol or a type/record has been JIT loaded and a namespace that needs the protocol/type/record class is being AOT compiled later. Since the namespace defining the class has already been loaded the class is never emitted on disk.

Comment by Nicola Mometto [ 06/Dec/14 6:51 PM ]

I've attached a tentative patch fixing the issue in the only way I found reasonable: forcing the reloading of namespaces during AOT compilation if the compiled classfile is not found in the compile-path or in the classpath

Comment by Nicola Mometto [ 06/Dec/14 7:30 PM ]

Updated patch forces reloading of the namespace even if a classfile exists in the compile-path but the source file is newer, mimicking the logic of clojure.lang.RT/load

Comment by Nicola Mometto [ 06/Dec/14 7:39 PM ]

Further testing demonstrated that this bug is not only scoped to deftypes/defprotocols but can manifest itself in the general case of a namespace "a" requiring a namespace "b" already loaded, and AOT compiling the namespace "a"

Comment by Tassilo Horn [ 08/Dec/14 4:46 AM ]

I'm also affected by this bug. Is there some workaround I can apply in the meantime, e.g., by dictating the order in which namespaces are going to be loaded/compiled in project.clj?

Comment by Nicola Mometto [ 15/Dec/14 10:58 AM ]

Tassilo, if you don't have control over whether or not a namespace that an AOT namespace depends on has already been loaded before compilation starts, requiring those namespaces with :reload-all should be enough to work around this issue

Comment by Tassilo Horn [ 15/Dec/14 11:36 AM ]

Nicola, thanks! But in the meantime I've switched to using clojure.java.api and omit AOT-compilation. That works just fine, too.

Comment by Michael Blume [ 15/Dec/14 5:05 PM ]

Tassilo, that's often a good solution, another is to use a shim clojure class

(ns myproject.main-shim (:gen-class))

(defn -main [& args]
  (require 'myproject.main)
  ((resolve 'myproject.main) args))

then your shim namespace is AOT-compiled but nothing else in your project is.

Comment by Tassilo Horn [ 16/Dec/14 1:07 AM ]

Thanks Michael, that's a very good suggestion. In fact, I've always used AOT only as a means to export some functions to Java-land. Basically, I did as you suggest but required the to-be-exported fn's namespace in the ns-form which then causes AOT-compilation of that namespace and its own deps recursively. So your approach seems to be as convenient from the Java side (no need to clojure.java.require `require` in order to require the namespace with the fn I wanna call ) while still omitting AOT. Awesome!

Comment by Nicola Mometto [ 06/Jan/15 6:07 PM ]

I'm marking this as incomplete to prevent further screening until the bug reported here: http://dev.clojure.org/jira/browse/CLJ-1620?focusedCommentId=37232&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-37232 is figured out

Comment by Nicola Mometto [ 07/Jan/15 4:43 AM ]

Fixed the patch, I'm re marking the tickets as Vetted as it was before.

Comment by Alex Miller [ 16/Jan/15 12:54 PM ]

This patch is being rolled back for 1.7.0-alpha6 pending further investigation into underlying problems and possible solutions.

Comment by Colin Fleming [ 19/Jan/15 4:41 AM ]

I'm not 100% sure, but this looks a lot like Cursive issue 369. It had a case that I could reproduce with JDK 7 but not JDK 8, has the same mysterious missing namespace class symptom, and involves mixed AOT/non-AOT namespaces. However it's happening at runtime, not at compile time, which doesn't seem consistent.

Comment by Alex Miller [ 19/Jan/15 7:29 AM ]

My error report above was incorrectly tied to this issue (see CLJ-1636). I will delete the comment.

Comment by Nicola Mometto [ 29/Jan/15 12:23 PM ]

Since ticket CLJ-1641 has been closed, I'll repost here a comment I posted in that ticket + the patch I proposed, arguing why I think the patch I proposed for this ticket should not have been reverted:

Zach, I agree that having different behaviour between AOT and JIT is wrong.

But I also don't agree that having clojure error out on circular dependencies should be considered a bug, I would argue that the way manifold used to implement the circular dependency between manifold.stream and manifold.stream.graph was a just a hack around lack of validation in require.

My proposal to fix this disparity between AOT and JIT is by making require/use check for circular dependencies before checking for already-loaded namespaces.

This way, both under JIT and AOT code like

(ns foo.a (:require foo.b))
(ns foo.b)
(require 'foo.a)

will fail with a circular depdenency error.

This is what the patch I just attached (0001-CLJ-1641disallow-circular-dependencies-even-if-the.patch) does.

Comment by Stuart Halloway [ 15/Aug/16 1:35 PM ]

This is actually a runtime problem, not a compilation problem.

AOTed Clojure code cannot see JITed Clojure classes (vars are fine), because of the rules of Java classloading. JITed code is loaded by a DynamicClassLoader, which delegates up to the classpath loader for AOTed code.

The person who runs a particular Clojure app can solve this problem by making sure their own consumption of AOT compilation is "infectious", i.e. if you want to AOT-compile library A which uses library B, then you need to AOT-compile library B as well.

I think that attempts to have the Clojure compiler magically implement the "infectious" rule above will cause more problems than they solve, and that we should close this ticket and provide good guidance for tools like lein and boot.

Comment by Michael Sperber [ 15/Aug/16 2:24 PM ]

This problem occurs within the compilation of a single library/project, so I don't think this can be solved by simple usages of Leiningen or Boot.

Comment by John Szakmeister [ 27/Oct/16 4:54 AM ]

I just spent quite a bit of time tracking down what I thought might be a variant of this problem. I've been trying to use Colin Fleming's new gradle-clojure plugin and was running into issues with the resulting shadow jar (the equivalent of an uberjar). At the time I believed it to be a problem in the gradle-clojure plugin, but it turned out to be a different issue. The shadow plugin was not preserving the last modified time on files extracted from dependencies, and it resulted in some source files looking newer than the class files. I suspect that Clojure was then recompiling the class, thinking it was out-of-date. This was nasty to track down and quite unexpected, but I can see the sense in the behavior now that I know what's going on. I'm not sure if anything should be done on the Clojure side, but it points to a problem with including the source and AOT files together--you really need to make sure the timestamps are kept intact and I can see that fact being easily overlooked. In this case, it was a plugin completely unrelated to Clojure that had to be fixed. I should also add that taking the infectious approach Stuart mentions is probably an issue in the Gradle and (possibly?) Maven environments, since there are separate plugins for packaging the uberjar.

FWIW, I have a fair amount of information in the ticket for gradle-clojure about the failure mode and the steps I went through to try and track down the problem: https://github.com/cursive-ide/gradle-clojure/issues/8. Also, I've put a pull request in on the shadow plugin to help keep the timestamps intact: https://github.com/johnrengelman/shadow/pull/260. There is also an issue in the shadow plugin describing the problem too: https://github.com/johnrengelman/shadow/issues/259.

Comment by Mike Rodriguez [ 14/Mar/17 6:15 PM ]

I'm confused by http://dev.clojure.org/jira/browse/CLJ-1544?focusedCommentId=36734&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-36734

Isn't the issue just that the "clj" source files also need to be included on the runtime classpath in order to be JITed when needed - since they were not AOTed?

i.e.

cp src/myrecord.clj target/

java -cp target:clojure.jar clojure.main -e "(use 'core)"

Why would the expectation be that "target" contains everything needed by the classpath when the AOT compilation was not done on everything? I don't get a failure when I add this step here.

The AOTed code looks fine. It just calls a `require` on the `myrecord` ns to JIT it. So I'm guessing I'm not seeing something else about this general problem statement.





[CLJ-2126] Can set! to fields of a defrecord Created: 14/Mar/17  Updated: 14/Mar/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: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler

Attachments: Text File 0001-CLJ-2126-don-t-assign-final-fields.patch    
Patch: Code
Approval: Triaged

 Description   

It is possible to set! to fields of a defrecord even though they are final.

(defprotocol SetA (seta [x a]))
=> SetA
(defrecord X [a]
  SetA
  (seta [this newa]
    ; Next line should error at compile time, does not.
    ; (However (set! a newa) does error correctly.)
    (set! (.a this) newa)))
=> user.X
(def x (->X 0))
=> #'user/x
x
=> #user.X{:a 0}
(seta x 1) ;; This should not run.
=> 1
x
=> #user.X{:a 1}

There are two issues here:

  1. The Clojure compiler does not detect that (set! (.a this) x) is assignment to a final field. This could be enhanced. Nicola Mometto has discovered why and believes he has a straightforward patch.
  2. The JVM bytecode verifier only performs the necessary final-assignment check on classfiles version 9 and above: https://bugs.openjdk.java.net/browse/JDK-8159215 (Clojure generates version 6 classfiles.) This is out of our hands.

Approach: make the compiler fail at compile time if trying to set! a field that's final

Patch: 0001-CLJ-2126-don-t-assign-final-fields.patch






[CLJ-1793] Reducer instances hold onto the head of seqs Created: 05/Aug/15  Updated: 14/Mar/17

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Alex Miller
Resolution: Unresolved Votes: 5
Labels: compiler
Environment:

1.8.0-alpha2 - 1.8.0-alpha4


Attachments: Text File 0001-Clear-this-before-calls-in-tail-position.patch     Text File clj-1793-2.patch     Text File clj-1793-3.patch    
Patch: Code and Test
Approval: Ok

 Description   

(This ticket started life as CLJ-1250, was committed in 1.8.0-alpha2, pulled out after alpha4, and this is the new version that fixes the logic about whether in a tail call as well as addresses direct linking added in 1.8.0-alpha3.)

Problem: Original example was with reducers holding onto the head of a lazy seq:

(time (reduce + 0 (map identity (range 1e8))))    ;; works
(time (reduce + 0 (r/map identity (range 1e8))))  ;; oome from holding head of range

Trickier example from CLJ-1250 that doesn't clear `this` in nested loop:

(let [done (atom false)
      f (future-call
          (fn inner []
            (while (not @done)
              (loop [found []]
                (println (conj found 1))))))]
  (doseq [elem [:a :b :c :done]]
    (println "queue write " elem))
  (reset! done true)
  @f)

Problem: #'reducer closes over a collection in order to reify CollReduce, and the closed-over collection reference is never cleared. When code attempts to reduce over this anonymous transformed collection, it will realize the tail while the head is stored in the closed-over.

Approach: When invoking a method in a tail call, clear 'this' prior to invoking.

The criteria for when a tail call is a safe point to clear 'this':

1) Must be in return position
2) Not in a try block (might need 'this' during catch/finally)
3) Not direct linked

Return position (#1) isn't simply (context == C.RETURN) because loop bodies are always parsed in C.RETURN context

A new dynvar METHOD_RETURN_CONTEXT tracks whether an InvokeExpr in tail position can directly leave the body of the compiled java method. It is set to RT.T in the outermost parsing of a method body and invalidated (set to null) when a loop body is being parsed where the context for the loop expression is not RETURN parsed. Added clear in StaticInvokeExpr as that is now a thing with direct linking again.

Removes calls to emitClearLocals(), which were a no-op.

Patch: clj-1793-3.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 05/Aug/15 12:16 PM ]

The this ref is cleared prior to the println, but the next time through the while loop it needs the this ref to look up the closed over done field (via getfield).

Adding an additional check to the inTailCall() method to not include tail call in a loop addresses this case:

static boolean inTailCall(C context) {
-    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null);
+    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null) && (LOOP_LOCALS.deref() == null);
}

But want to check some more things before concluding that's all that's needed.

Comment by Alex Miller [ 05/Aug/15 1:36 PM ]

This change undoes the desired behavior in the original CLJ-1250 (new tests don't pass). For now, we are reverting the CLJ-1250 patch in master.

Comment by Ghadi Shayban [ 05/Aug/15 3:12 PM ]

Loop exit edges are erroneously being identified as places to clear 'this'. Only exits in the function itself or the outermost loop are safe places to clear.

Comment by Ghadi Shayban [ 05/Aug/15 8:43 PM ]

Patch addresses this bug and the regression in CLJ-1250.

See the commit message for an extensive-ish comment.

Comment by Alex Miller [ 18/Aug/15 12:33 PM ]

New patch is same as old, just adds jira id to beginning of commit message.

Comment by Rich Hickey [ 24/Aug/15 10:00 AM ]

Not doing this for 1.8, more thought needs to go into whether this is the right solution to the problem. And, what is the problem? This title of this patch is just something to do.

Comment by Alex Miller [ 24/Aug/15 10:21 AM ]

changing to vetted so this is at a valid place in the jira workflow

Comment by Ghadi Shayban [ 24/Aug/15 10:45 AM ]

Rich the original context is in CLJ-1250 which was a defect/problem. It was merged and revert because of a problem in the impl. This ticket is the continuation of the previous one, but unfortunately the title lost the context and became approach-oriented and not problem-oriented. Blame Alex. (I kid, it's an artifact of the mutable approach to issue management.)

Comment by Nicola Mometto [ 23/Mar/16 7:34 AM ]

Just a note that the original ticket for this issue had 10 votes

Comment by Nicola Mometto [ 30/Mar/16 8:50 AM ]

The following code currently eventually causes an OOM to happen, the patch in this ticket correctly helps not holding onto the collection and doesn't cause memory to run infinitely

Before patch:

user=> (defn range* [x] (cons x (lazy-seq (range* (inc x)))))
#'user/range*
user=> (reduce + 0 (eduction (range* 0)))
OutOfMemoryError Java heap space  clojure.lang.RT.cons (RT.java:660)

After patch:

user=> (defn range* [x] (cons x (lazy-seq (range* (inc x)))))
#'user/range*
user=> (reduce + 0 (eduction (range* 0)))
;; runs infinitely without causing OOM
Comment by Alex Miller [ 08/Sep/16 5:14 PM ]

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

Comment by Alex Miller [ 14/Mar/17 11:42 AM ]

Looks like this one was missed when applying patches for release





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

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

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

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

 Description   

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

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

Patch: mvn3.patch

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






[CLJ-2062] Spec import and refer-clojure macros Created: 17/Nov/16  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: core.specs, spec

Attachments: Text File import-referclj-2.patch    
Patch: Code
Approval: Ok

 Description   

Add specs for import and refer-clojure.

Patch:

  • Fixes some indentation of previous specs
  • Factors out ::filters spec from ::ns-refer-clojure
  • Factors out ::import-list from ::ns-import
  • Reuses ::filters in ::ns-refer
  • Reuses ::filters in ::use-prefix-list
  • Removes :ret any? in ::ns-use (no need for it)
  • Adds clojure.core/import spec
  • Adds clojure.core/refer-clojure spec

Patch: import-referclj-2.patch






[CLJ-2043] s/form of conformer is broken Created: 14/Oct/16  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

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

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

 Description   

s/form of s/conformer is wrong:

(s/form (s/conformer str))
=> str

Proposed: Fix the form for conformer to match the conformer call.

Patch: clj-2043.patch






[CLJ-2035] Bad s/form for collection specs Created: 07/Oct/16  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

Type: Defect Priority: Major
Reporter: Johan Gall Assignee: Alex Miller
Resolution: Completed Votes: 6
Labels: spec

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

 Description   

There are several problems with s/form for collection specs (coll-of,map-of,every,every-kv):

1. coll spec forms expose implementation details of building on every:

(s/form (s/coll-of int?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval16$fn__18 0xd506900 "user$eval16$fn__18@d506900"] :clojure.spec/kind-form nil :clojure.spec/conform-all true)

2. form does not resolve nested spec preds:

(s/def ::a (s/every (s/tuple ::b)))

(s/form ::a)
=> (clojure.spec/every (*s/tuple* :user/b) [ ... ])

(which impacts map-of and coll-of).

3. :kind fn is not resolved

(s/form (s/coll-of int? :kind vector?))
=> (clojure.spec/every int? :clojure.spec/cpred #object[user$eval4$fn__6 0x8fc095 "user$eval4$fn__6@8fc095"] :clojure.spec/kind-form clojure.core/vector? :kind #object[clojure.core$vector_QMARK___6428 0x6596f6ef "clojure.core$vector_QMARK___6428@6596f6ef"] :clojure.spec/conform-all true)

Ignoring the rest of the problems from #1, the :kind should be here but should be the resolved form (clojure.core/vector?).

Patch: clj-2035-2.patch



 Comments   
Comment by Johan Gall [ 10/Mar/17 3:58 PM ]

Thanks a lot!





[CLJ-2114] ::defn-args spec incorrectly parses map body as a prepost rather than function body Created: 16/Feb/17  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

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

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

 Description   

Reported by Claire Alvis in #clojure-spec:

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

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

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

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

Screened by: Alex Miller



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

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





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

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

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

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

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

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

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

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

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

After:

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

Patch: CLJ-2100.patch






[CLJ-2055] binding-form spec parses symbol-only maps incorrectly Created: 08/Nov/16  Updated: 14/Mar/17  Resolved: 14/Mar/17

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: spec
Environment:

1.9.0-alpha14


Attachments: Text File CLJ-2055-01.patch    
Patch: Code
Approval: Ok

 Description   

The :clojure.core.specs/binding-form spec incorrectly treats some maps as sequential bindings.

Actual:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:seq {:elems [[:seq {:elems [[:sym x] [:sym y]]}]]}]

Expected:

user=> (s/conform :clojure.core.specs/binding-form '{x y})
[:map {x y}]

Cause:

When there is no :keys, :strs, or :syms from :clojure.core.specs/map-special-binding, then :clojure.core.specs/seq-binding-form treats a map as sequential.

Proposed fix:

Include an (s/and vector? ...) check. See patch.

Patch: CLJ-2055-01.patch
Screened by: Alex Miller






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

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

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

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

 Description   

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

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

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

Patch: clj-1743-2.patch

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

Demo.java
package clojure_static_initialization;

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

unknown.

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

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

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

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





[CLJ-994] repeat reducer Created: 11/May/12  Updated: 12/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Jason Jackson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reducers

Attachments: Text File 0001-repeat-for-clojure.core.reducers.patch    
Patch: Code and Test

 Description   

i'm working on clojure.core/repeat reducer.



 Comments   
Comment by Andy Fingerhut [ 17/May/12 6:18 PM ]

Jason, have you tried to build this using JDK 1.6.0? I've tried on Mac OS X 10.6.8 + Oracle/Apple JDK 1.6.0 and Ubuntu 11.10 + IBM JDK 1.6.0, and on both it compiles, but during the tests fails with a ClassNotFoundException for class jsr166y.ForkJoinTask.

It builds and tests cleanly on Ubuntu 11.10 + Oracle JDK 1.7.0 for me.

Comment by Jason Jackson [ 17/May/12 6:41 PM ]

That's an issue that applies to all of core.reducers. Alan Malloy experienced it as well. I tried fixing it, but eventually just upgraded to JDK 1.7. I don't understand why it's happening.

Comment by Jason Jackson [ 19/May/12 2:55 PM ]

This issue is isolated to mvn test afaik.

When I include clojure inside a leiningen project, and add jsr166y.jar to lib directory, core.reducers works fine with java 1.6.

Comment by Andy Fingerhut [ 20/May/12 3:00 AM ]

Jason, you say it applies to all of core.reducers in your May 17, 2012 comment. I don't understand. Without your patch applied, I can run "./antsetup.sh ; ant" in a freshly-pulled Clojure git repo on either of the JDK 1.6.0 versions mentioned in my earlier comment, and do not get any errors during the tests. Are you saying perhaps that core.reducers currently has no tests that exercise the problem now, but your patch adds such tests that fail, even with no other changes to the code?

Comment by Jason Jackson [ 20/May/12 11:55 AM ]

Yah that's right. Now that you mention it, my patch is the first unit test to call r/fold (the existing tests do non-parallel reductions).

Comment by Andy Fingerhut [ 08/Jun/12 7:11 PM ]

With Stuart Halloway's commit to Clojure master on June 8, 2012 titled "let reducers tests work under ant", patch 0001-repeat-for-clojure.core.reducers.patch dated May 11, 2012 now runs correctly even the new unit tests requiring class jsr166y.ForkJoinTask with Oracle/Apple JDK 1.6 and Linux IBM JDK 1.6.

Comment by Jason Jackson [ 14/Aug/12 1:17 AM ]

I'm on the contributors list. Is this patch still needed?
sorry for long long delay.

Comment by Jason Jackson [ 14/Sep/12 2:37 PM ]

This patch should wait until http://dev.clojure.org/jira/browse/CLJ-993 is committed. I think there's a some shared code.

Comment by Ghadi Shayban [ 09/Jan/16 5:24 PM ]

repeat is now reducible in 1.7.0

Comment by Ghadi Shayban [ 29/Jun/16 9:05 PM ]

Can we close this ticket?

Comment by Alex Gunnarson [ 12/Mar/17 9:49 PM ]

I think it would be nice to have this reducer included for purposes of parallelism, or else extend `CollFold` to `clojure.lang.Repeat` (which is perhaps cleaner).





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

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

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


 Description   

fspec doesn't generate pure functions.

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

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

(stest/check `foo)

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

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

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

(stest/check `bar)

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



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

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





[CLJ-2124] Catch multiple exceptions in a single catch block Created: 12/Mar/17  Updated: 12/Mar/17

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

Type: Enhancement Priority: Minor
Reporter: Daniel Compton Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: exceptions, try-catch

Approval: Triaged

 Description   

Java 7 and up support multi-catch exceptions (http://www.oracle.com/technetwork/articles/java/java7exceptions-486908.html). It would be handy if Clojure also supported them to prevent catching something like Exception and then writing manual logic to check the Exception type, or duplicating the logic over multiple catch blocks.

A possible syntax for this could be:

(try (fn-that-throws)
     (catch (UnknownHostException NoRouteToHostException) e
       (go-offline)))

Prior art for this is a try* macro: https://gist.github.com/Gonzih/5814945.

One nuance to handle is

Edit: Note that in Java 7, you cannot both catch ExceptionA& ExceptionB in the same time, if ExceptionB is inherited(directly or indirectly) from ExceptionA. Compiler will complain: The exception ExceptionB is already caught by the alternative ExceptionA. - http://stackoverflow.com/a/3495968/826486

I tried searching to see if this had been asked already, but got mountains of results. I didn't see anything in the first few pages though.






[CLJ-2030] Auto-create alias namespaces Created: 28/Sep/16  Updated: 10/Mar/17  Resolved: 10/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Herwig Hochleitner Assignee: Unassigned
Resolution: Declined Votes: 8
Labels: keywords, namespace, portability, spec

Attachments: Text File clj-2030-2.patch     Text File clj-2030-3-1.patch     Text File clj-2030-3-2.patch     Text File clj-2030.patch    
Patch: Code and Test
Approval: Triaged

 Description   

It is useful to name keywords in namespaces, without creating or requiring those namespaces. When wanting to do that via an ::alias/keyword, the aliased namespace has to actually exist, in order to be aliased.
Currently, in Clojure, this can be achieved dynamically, through a combination of create-ns and alias, Clojurescript requires a dummy file and a :require :as.

Proposals:

1. Extend clojure.core/alias to auto-create missing namespaces
2. Extend clojure.core/alias to accept varargs & {:as kvs}
3. Extend ns to accept (:alias ...) clauses

Patch:

  • clj-2030.patch does 1 (but not 2 or 3), and was screened by SDH
  • clj-2030-2.patch does 1+2 (but not 3)
  • clj-2030-3-1.patch does 1+3 (but not 2)
  • clj-2030-3-2.patch does 1+2+3

Before:

user=> (alias 'parts 'company.domain.parts)
java.lang.Exception: No namespace: company.domain.parts found

After:

user=> (alias 'parts 'company.domain.parts)
nil
user=> ::parts/widget
:company.domain.parts/widget


 Comments   
Comment by Alex Miller [ 28/Sep/16 10:04 PM ]

From original description:

My use case is a simplification of data.xml, which would benefit greatly from a uniform way to alias + auto-create namespaces within the ns clause.

I would like to support the syntax:

(ns foo.bar
  (:alias xh  #xml/ns "http://..<xhtml>.."
          svg #xml/ns "http://..<svg>.."))

{:tag ::xh/div
 :content [{:tag ::svg/g}]}

see https://github.com/bendlas/data.xml/commit/22cbe21181175d302c884b4ec9162bd5ebf336d7

Comment by Alex Miller [ 28/Sep/16 10:08 PM ]

Thanks for filing this, it is something we've looked at a bit already. I simplified the description a bit and moved the use case and syntax to comments. I don't really understand the ns :alias example given in your syntax proposal but I think it's very unlikely we would go that far.

Comment by Herwig Hochleitner [ 29/Sep/16 3:55 AM ]

My syntax example could already be implemented, if alias had the proposed behavior and was available in an ns clause. In the linked commit, I implemented a scheme to encode xml namespaces in clojure namespaces, by using percent-encoding. I could easily provide that reader tag, if clojure and clojurescript provided the proposed extensions to alias and ns.

Comment by Alex Miller [ 29/Sep/16 8:53 AM ]

Yeah, I get it now (sleep!). I think the particular example is distracting to understand the enhancement request though.

Comment by Alex Miller [ 18/Oct/16 9:13 AM ]

moving this to vetted just so we don't lose track of it, but Rich has not actually ok'ed this for 1.9 yet

Comment by Herwig Hochleitner [ 06/Dec/16 10:50 PM ]

added patches for 3

Comment by Alex Miller [ 10/Mar/17 8:07 AM ]

Based on conversations to date, we don't intend to make this change, although there are some other ideas in work.

Comment by Alex Miller [ 10/Mar/17 8:13 AM ]

See CLJ-2123 for tracking of alternative.

Comment by Brandon Bloom [ 10/Mar/17 7:07 PM ]

Could you share some design insights please?

My initial thought is that auto-creating a namespace would interact poorly with requiring namespaces and code loading as currently implemented.

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

Well you can look at the patch for the approach to what is suggested in this ticket. When you try to create an alias that isn't loaded, simply find or create it. If it does exist, it will be loaded. If it does not, then it will be created.

But kind of a moot point as we're not going to do this.





[CLJ-1860] 0.0 and -0.0 compare equal but have different hash values Created: 01/Dec/15  Updated: 10/Mar/17

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

Type: Defect Priority: Minor
Reporter: Patrick O'Brien Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math

Attachments: Text File clj-1860-2.patch     Text File clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch     Text File CLJ-1860-negative-zero-hash-eq-fix.patch    
Patch: Code and Test
Approval: Screened

 Description   

0.0 and -0.0 compare as equal but have different hash values:

user=> (= 0.0 -0.0)
true
user=> (hash -0.0)
-2147483648
user=> (hash 0.0)
0

This causes problems as the equality/hashing assumption is violated.

user=> #{[1 2 0.0] [1 2 -0.0]}
#{[1 2 -0.0] [1 2 0.0]}

user=> (hash-map 0.0 1 -0.0 2)
{0.0 2}

user=> (hash-map [0.0] 1 [-0.0] 2)
{[0.0] 1, [-0.0] 2}

user=> (array-map [0.0] 1 [-0.0] 2)
{[0.0] 2}

user=> (hash-set [0.0] [-0.0])
#{[0.0] [-0.0]}

Cause: The source of this is due to some differences in Java. Java primitive double 0.0 and -0.0 == but the boxed Double is NOT .equals(). See also: http://docs.oracle.com/javase/7/docs/api/java/lang/Double.html#equals%28java.lang.Object%29

Double equality is checked with == in Clojure, which will report true. Hashing falls through to .hashCode(), which returns different values (but is consistent with the .equals() result on the boxed form).

Approach: While there are times when 0.0 and -0.0 being different are useful (see background below), most Clojure users expect these to compare equal. IEEE 754 says that they should compare as equals as well. So the approach to take here is to leave them as equal but to modify the hash for -0.0 to be the same as 0.0 so that `=` and `hash` are consistent. The attached patch takes this approach.

Patch: clj-1860-2.patch

Screened: Alex Miller

Alternative: Make 0.0 != -0.0. This approach affects a much larger set of code as comparison operators etc may be affected. The patch clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch may be one way to implement this approach, and seems fairly small in the quantity of code affected (2 methods).

Background: https://en.wikipedia.org/wiki/Signed_zero



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

Just to summarize, it seems like this functionality in Clojure is the same as it is in Java:

0.0 == -0.0: true
new Double(0.0).hashCode(): 0
new Double(-0.0).hashCode(): -2147483648
new Double(-0.0).equals(new Double(0.0)): false

I can see pros and cons to both of the aforementioned approaches, as well as just leaving this one be. Does anyone else have any input on this one? Is this issue something we should rectify, or by changing it will we end up creating more problems than we solved?

Comment by Andy Fingerhut [ 10/Feb/16 12:38 AM ]

The Java behavior you demonstrate shows that in this case, they are not equals, so there is no need for the hashCode() values to be the same in order to satisfy the hash consistency property of equals and hashCode.

Clojure currently violates the hash consistency property that should ideally hold between clojure.core/= and clojure.core/hash, for 0.0 and -0.0.

Changing clojure.core/= so it is false would restore the hash consistency property for these values. Keeping (clojure.core/== 0.0 -0.0) true is hopefully something that will be maintained across any change, but that does not violate hash consistency, because that property has nothing to say about the value of clojure.core/==

Comment by Stephen Hopper [ 10/Feb/16 7:10 AM ]

Thanks for the explanation, Andy. That makes sense to me. I'll put together some tests and a possible solution for evaluation.

Comment by Stephen Hopper [ 10/Feb/16 11:20 AM ]

After diving into the source code a bit more, my preference is to modify the hash calculation for -0.0 to be the same as the hash calculation for 0.0. This will restore the hash consistency property without breaking other mathematical operations. Basically, if we update clojure.core/= to return false for (= 0.0 -0.0), we will need to update other functions (clojure.core/<, clojure.core/>, etc.) so that -0.0 and 0.0 still follow basic numerical properties surrounding equality and ordering. I'll add some tests and a possible patch to hash calculation for numbers for consideration.

Comment by Andy Fingerhut [ 10/Feb/16 12:40 PM ]

Someone, perhaps Patrick O'Brien , brought up a preference that making (= 0.0 -0.0) false would help in some applications, e.g. numerical applications involving normal vectors where it was beneficial if (= 0.0 -0.0) was false. I have no knowledge whether this preference will determine what change will be made to Clojure, if any. If someone finds a link to the email discussion that was in one of the Clojure or Clojure Dev Google groups, that would be a useful reference.

Comment by Stephen Hopper [ 11/Feb/16 9:47 PM ]

I've added a pair of patches for review on this one (one contains test updates and the other contains my proposed updates to the hash calculation). I realize that this is different than the approach proposed earlier in the ticket, but I think it should be the preferred approach. As I mentioned earlier, merely changing clojure.core/= to return false for (= 0.0 -0.0) would require also updating other numeric equality functions (clojure.core/<, clojure.core/>, etc.). More importantly, I feel that this behavior would be different than what most Clojure developers would expect.

For this reason, the patches I've updated merely modify the calculation of hashes with Numbers.java for Floats and Doubles for which isZero returns true to return the hashCode for positive 0.0 instead of negative 0.0.

Is this acceptable?

EDIT:
With these patches applied, we get the following behavior in the REPL:

user=> (= 0.0 -0.0)
true
user=> (hash 0.0)
0
user=> (hash -0.0)     
0
user=> (hash (float 0.0))
0
user=> (hash (float -0.0))
0
Comment by Andy Fingerhut [ 12/Feb/16 3:48 AM ]

Stephen, please see here http://dev.clojure.org/display/community/Developing+Patches for the commands used to create patches in the desired format.

Only a screener or Rich can say whether the patch is acceptable in the ways that matter for committing into Clojure.

Comment by Stephen Hopper [ 12/Feb/16 7:42 AM ]

I've corrected the format on my patch files and resubmitted them as one patch. Let me know if you see any issues with this. Also, it's worth pointing out that the first two patch files can be ignored / deleted.

Comment by Andy Fingerhut [ 14/Feb/16 1:30 PM ]

There are also instructions on that page for deleting old attachments, in the section titled "Removing patches", if you wished to do that.

A very minor comment, as the email address you use in your patches is completely up to you (as far as I know), but the one you have in your patch doesn't look like one that others could use to send you a message. If that was intentional on your part, no problem.

Comment by Stephen Hopper [ 14/Feb/16 1:48 PM ]

I've corrected the email address snafoo and removed the outdated patches. Thanks for walking me through this stuff, Andy.

Comment by Steve Miner [ 23/Feb/16 3:43 PM ]

The suggested fix is to make (hash -0.0) return the same as (hash 0.0). With the proposed patch, both will return 0. That happens to be the same as (hash 0). Not wrong at all, but maybe slightly less good than something else. Since you're making a change anyway, why not go the other way and use the -0.0 case as the common result?

I'm thinking that it would be a useful property if the hash of a long N is not equal to the hash of the corresponding (double N). In Clojure 1.8, zero is the only value I could quickly find where the long and the double equivalents have the same hash value.

The patch could be slightly tweaked to make (hash 0.0) and (hash -0.0)

return new Double(-0.0).hashCode()

The only change to the patch is to add the negative sign. The new hash result is -2147483648.

Admittedly this is an edge case, not a real performance issue. People probably don't mix longs and doubles in sets anyway. On the other hand, zeroes are kind of common. Since you're proposing a change, I thought it's worth considering a slight tweak.

Comment by Steve Miner [ 23/Feb/16 3:46 PM ]

Same for the float case, of course.

Comment by Alex Miller [ 24/Feb/16 1:36 PM ]

Looking at this agin, I'm ok with the approach and agree it's definitely a smaller change. Few additional changes needed in the patch and then I'll move it along:

  • Rather than `new Double(0.0).hashCode()` and `new Float(0.0).hashCode()`, move those values into `private static final int` constants and just return them.
  • In the comparison tests, throw -0.0M in there as well
  • Squash the patch into a single commit

Re Steve's suggestion, I do not think it's critical that long and double 0 hash differently and would prefer that double hashes match Java hashCode, so I would veto that suggestion.

Comment by Stephen Hopper [ 24/Feb/16 1:52 PM ]

Thank you, Alex. I'll make those updates.

Comment by Stephen Hopper [ 24/Feb/16 2:43 PM ]

Alex, I've attached the new patch file (CLJ-1860-negative-zero-hash-eq-fix.patch) to address the points from your previous post. Let me know if I missed anything.

Thanks!

Comment by Mike Anderson [ 25/Feb/16 7:44 PM ]

I may be late to the party since I have only just seen this, but I have a strong belief that 0.0 and -0.0 should be == but not =.

Reasons:

  • Anyone doing numerical comparison should use ==, so you want the result to be true
  • Anyone doing value comparison should use =, so you want the result to be false because these are different IEE784 double values. This includes set membership tests etc.

i.e. the Java code is doing it right, and we should be consistent with this.

Comment by Alex Miller [ 25/Feb/16 11:52 PM ]

I think there is consensus that == should be true, so we can set that aside and focus on =.

The reality is that there is no easy way to compare two collections with == (for example comparing [5.0 0.0 1.0] and [5.0 -0.0 1.0]). This is an actual use case that has been problematic for multiple people. While I grant there are use cases where 0.0 and -0.0 are usefully differentiable, I do not know of a real case in the community where this is the desired behavior, so I would rather err on the side of satisfying the intuition of the larger (and currently affected) population.

Also note that the Java code is doing it BOTH ways (primitive doubles are equal, boxed doubles are not), so I think that's a weak argument.

Comment by Alex Miller [ 26/Feb/16 8:44 AM ]

(after sleeping more on this...) It's possible that a better answer here is to expand what can be done with ==. I trust that when Rich looks at this ticket he will have his opinions which may or may not match up to mine and if so, we'll go in a different direction.

Comment by Andy Fingerhut [ 27/Feb/16 10:15 AM ]

Attachment clj-1860-make-equals-false-for-pos-neg-0.0-v1.patch dated Feb 27 2016 is a first cut at implementing a change where = returns false when comparing positive and negative 0.0, float or double.

As far as I can tell, there is no notion of positive and negative 0 for BigDecimal, so no change in behavior there.

Comment by Alex Miller [ 17/Nov/16 4:16 PM ]

Rich says: "0.0=-0.0, make hash the same"





[CLJ-2076] s/coll-of and s/map-of do not unform their elements Created: 05/Dec/16  Updated: 10/Mar/17

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

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

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

 Description   

s/coll-of and s/map-of unform with identity but should unform their elements:

(s/def ::o (s/coll-of (s/or :i integer? :s string?)))
(->> [1 2 "blah"] (s/conform ::o) (s/unform ::o))
=> [[:i 1] [:i 2] [:s "blah"]]

Expected: [1 2 "blah"]

Cause: every-impl unform* just returns x

Approach: Use the init/add/complete fns to generate an unformed value (when needed)

Patch: clj-2076-2.patch



 Comments   
Comment by Alex Miller [ 07/Dec/16 5:50 PM ]

This needs tests and a bunch of verification, but first pass at fixing.

Comment by Alex Miller [ 09/Dec/16 8:03 AM ]

Added tests, ready to screen





[CLJ-2068] s/explain of evaluated predicate yields :s/unknown Created: 23/Nov/16  Updated: 10/Mar/17

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

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

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

 Description   

Got:

(s/explain #{1 2 3} 4)
val: 4 fails predicate: :clojure.spec/unknown

(s/explain odd? 10)
val: 10 fails predicate: :clojure.spec/unknown

Expected to receive a description of the failing predicate as in:

(s/def ::s #{1 2 3})
(s/explain ::s 4)
;; val: 4 fails spec: :user/s predicate: #{1 3 2}

(s/def ::o odd?)
(s/explain ::o 10)
val: 10 fails spec: :user/o predicate: odd?

Cause: specize was falling through on these cases to Object and just returning unknown.

Proposed:
Special handling for 2 cases:
1. Sets - explictly catch IPersistentSet and use the set as the form.
2. Functions - demunge the function name and use the qualified function name symbol as the form. Add a special check for anonymous functions and revert to ::unknown for those (not much we can do with an eval'ed anonymous function).

Patch: clj-2068-2.patch



 Comments   
Comment by Alex Miller [ 13/Dec/16 6:52 PM ]

Simplified anon fn check and added a few basic tests.

Comment by Stuart Halloway [ 10/Mar/17 11:22 AM ]

Could the Specize protocol be extended to IFn, reducing the iffiness?

Comment by Alex Miller [ 10/Mar/17 11:39 AM ]

Yes, I think that would make sense.

Comment by Ghadi Shayban [ 10/Mar/17 12:57 PM ]

Extending Specize to IFn may incur the wrath of CLJ-1152





[CLJ-2123] Lighter-weight aliasing for keywords Created: 10/Mar/17  Updated: 10/Mar/17

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: keywords, namespace

Approval: Vetted

 Description   

It is useful to make qualified keywords, and particularly so with spec. Using namespace aliases helps a lot in working with a lot of qualified keywords. However, currently creating an aliased namespace requires that the namespace actually exists.

This ticket is a placeholder to do something more with lighter-weight aliasing for keywords. Details TBD.






[CLJ-2085] Add additional info to explain-data to help explain printers Created: 15/Dec/16  Updated: 10/Mar/17

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

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

Attachments: Text File explain-data.patch    
Patch: Code
Approval: Screened

 Description   

Right now, the explain-data provided to the explain printer function only has the list of problem data. There are many interesting things a printer could do with access to the root spec and value but those are not currently available.

Proposed: Provide these values in the explain-data map (as ::spec and ::value).

Patch: explain-data.patch






[CLJ-2065] reduce-kv fails on subvec Created: 20/Nov/16  Updated: 09/Mar/17

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

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Approval: Triaged

 Description   

reduce-kv works as expected on vectors with the element index passed as the "key" argument. However, it fails with a subvec because clojure.lang.APersistentVector$SubVector does not implement IKVReduce.

(reduce-kv + 0 [1 2 3])
9

(reduce-kv + 0 (subvec [1 2 3] 1))
IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)

One work around is to copy the subvec into a vector:

(reduce-kv + 0 (into [] (subvec [1 2 3] 1)))
6

Note however, the `vec` would not work here. Since Clojure 1.7, vec will return a subvec rather than copying.

(reduce-kv + 0 (vec (subvec [1 2 3] 1)))
IllegalArgumentException No implementation of method: :kv-reduce of protocol: #'clojure.core.protocols/IKVReduce found for class: clojure.lang.APersistentVector$SubVector clojure.core/-cache-protocol-fn (core_deftype.clj:583)



 Comments   
Comment by Steve Miner [ 20/Nov/16 12:53 PM ]

Here is my current work-around:

(extend-type clojure.lang.APersistentVector$SubVector
  clojure.core.protocols/IKVReduce
  (kv-reduce [subv f init]
    (transduce (map-indexed vector)
               (fn ([ret] ret) ([ret [k v]] (f ret k v)))
               init
               subv)))

In my tests it was usually faster to copy the subvec into a regular vector but I like the look of the transduce fix. It would probably be faster to add a native Java implementation in APersistentVector.java. I'm willing to do the work if the Clojure/core team wants a patch.

Comment by Steve Miner [ 09/Mar/17 1:38 PM ]

Revised work-around using more interop for better performance. Comparable to the speed of normal vector reduce-kv.

(when-not (satisfies?   clojure.core.protocols/IKVReduce (subvec [1] 0))
  (extend-type clojure.lang.APersistentVector$SubVector
    clojure.core.protocols/IKVReduce
    (kv-reduce
      [subv f init]
      (let [cnt (.count subv)]
        (loop [k 0 ret init]
          (if (< k cnt)
            (let [val (.nth subv k)
                  ret (f ret k val)]
              (if (reduced? ret)
                @ret
                (recur (inc k) ret)))
            ret))))))




[CLJ-2041] clojure.spec/keys requires input collections conform to clojure.core/map? Created: 11/Oct/16  Updated: 08/Mar/17

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

Type: Defect Priority: Minor
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: spec

Approval: Vetted

 Description   

I would like to use specs to validate Datomic entities. However, `s/keys` is too restrictive in that it requires input collections to conform to `clojure.core/map?` instead of some more primitive interface (for example clojure.lang.ILookup or clojure.lang.Associative).



 Comments   
Comment by Alex Miller [ 04/Nov/16 8:21 AM ]

s/keys uses IPersistentMap's Iterable support for iterating through all entries for validation. ILookup and Associative do not support iteration. So, that's why it is the way it is. But, understand the desire.

Comment by Alex Miller [ 04/Nov/16 8:34 AM ]

Datomic entities are seqable so maybe that's a potential path (would be slower for actual PHMs though).

Comment by Odin Standal [ 04/Nov/16 8:35 AM ]

Thanks for following up. So any ideas or guidance on how to use clojure.spec with Datomic entities?

Comment by Alex Miller [ 04/Nov/16 8:37 AM ]

For now, you could use into to pour an entity into a PHM before validating. I hesitate to suggest it, but that could even be in the spec with a leading conformer.

Comment by Alex Miller [ 14/Nov/16 12:16 PM ]

Moving this into 1.9 for the moment just so we don't lose it. Not sure whether we can or will actually do anything with this though.

Comment by Alex Miller [ 08/Mar/17 10:00 AM ]

This is related to CLJ-2080 as it's the same basic issue of what map-of and every-kv actually expect as valid inputs, which is: something that seqs to map entries. We can't really write a predicate for it without an interface "ISeqsToMapEntries" (intentionally bad name) to indicate this. java.util.Map, IPersistentMap, etc imply this, but an object can seq to map entries without meeting all the constraints of those much broader interfaces. ILookup does not and should not imply this.





[CLJ-2122] flatten docstring does not describe lazy result Created: 07/Mar/17  Updated: 07/Mar/17

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

Type: Enhancement Priority: Trivial
Reporter: Alan Thompson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring
Environment:

All


Approval: Triaged

 Description   

clojure.core/flatten uses tree-seq to return a lazy result. The lazy nature of the result is not described in the docstring.

Original docstring:
--------------------------------
"Takes any nested combination of sequential things (lists, vectors,
etc.) and returns their contents as a single, flat sequence.
(flatten nil) returns an empty sequence."

Proposed docstring:
--------------------------------
"Takes any nested combination of sequential things (lists, vectors,
etc.) and returns their contents as a single, flat lazy sequence.
(flatten nil) returns an empty sequence."



 Comments   
Comment by Alex Miller [ 07/Mar/17 6:28 PM ]

Seems reasonable.





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

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

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

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

 Description   

The docstring for `pos?` is as follows:

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

This should be either:

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

or

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


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

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

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

Patch welcome - change should update docstring, not arg.

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

Duplicate of CLJ-1859?

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

Closed as duplicate





[CLJ-1859] Update parameter name to reflect docstring Created: 30/Nov/15  Updated: 04/Mar/17

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

Type: Enhancement Priority: Trivial
Reporter: Matthew Boston Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: docstring

Attachments: Text File CLJ-1859-Update-parameter-name-to-reflect-docstring.patch    
Approval: Triaged

 Description   

The docstrings for `zero?`, `pos?`, and `neg?` reference `num` but the parameter is named `x`. This issue is to update the name of the parameter to `num` to reflect the docstring.



 Comments   
Comment by Alex Miller [ 30/Nov/15 1:14 PM ]

The inline fns should be updated too.

Comment by Matthew Boston [ 30/Nov/15 1:22 PM ]

Thanks, Alex. I was trying to follow the existing pattern that the inline functions have shorter parameter names. New patch attached.

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

CLJ-2121 should be closed as a duplicate of this, I assume?





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

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

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

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



 Description   

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

Simple example (1.7):

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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

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

;;Microbenchmarks.

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

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

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

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

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

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





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

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

Type: Enhancement Priority: Major
Reporter: Tommi Reiman Assignee: Unassigned
Resolution: Unresolved Votes: 5
Labels: spec
Environment:

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



 Description   

Problem

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

Details

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

Examples:

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

Here is a more complete example:

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

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

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

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

Suggestion

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

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

Using it would look like this:

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

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

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

Alternative

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

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



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

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

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

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

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

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

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





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

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

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

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



 Description   

I have the following file:

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

REPL is running in the same folder.
Calling

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

apple
orange
watermelon
(nil nil nil)

Calling

(load "for")
yields:

for.clj loaded
nil

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



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

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

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





[CLJ-1081] REPL binding not working that works with with-bindings Created: 30/Sep/12  Updated: 23/Feb/17  Resolved: 23/Feb/17

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

Type: Defect Priority: Minor
Reporter: Steven Devijver Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: repl


 Description   

This works as expected:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (eval '(clojure.repl/doc println))))"

Output:

"{:ns #<Namespace clojure.core>, :name println, :arglists ([& more]), :added \"1.0\", :static true, :doc \"Same as print followed by (newline)\", :line 3325, :file \"clojure/core.clj\"}"

But the same thing does not work in the REPL:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (clojure.main/repl :init (fn [] {#'clojure.repl/print-doc str}))))"

Output for Output of {{(doc println)}}:

user=> (doc println)
-------------------------
clojure.core/println
([& more])
Same as print followed by (newline)
nil
user=>




 Comments   
Comment by Steven Devijver [ 01/Oct/12 5:51 AM ]

Found a work-around:

java -jar clojure-1.4.0.jar -e "(do (require 'clojure.repl) (.setDynamic #'clojure.repl/print-doc) (with-bindings {#'clojure.repl/print-doc str} (clojure.main/repl)))))"

I'm still not sure whether the method above using :init should or should not work.

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

You shouldn't expect to be able to reconfigure the repl from within itself like this. I think the workaround of creating a new differently configured repl is correct.





[CLJ-929] Accessing Java property starting with _ has issues in 1.4 Created: 07/Feb/12  Updated: 23/Feb/17  Resolved: 23/Feb/17

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

Type: Defect Priority: Minor
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: defrecord

Approval: Triaged

 Description   

When attempting to use interop syntax with symbols which aren't legal Java names (such as deleted?), the names are mangled a bit. That's necessary, of course, and the method of munging can be internal to the compiler. However, the behavior when munging changed a little between 1.3 and 1.4 beta1. Obviously the specifics of munging are something I should avoid relying on, but the way it changed looks like an accident or a bug even so.

The use-case I ran into is that defrecords contain a field named __meta for tracking their metadata. In both 1.3 and 1.4 you can get at that field with (. record __meta), which avoids munging. But on 1.3 (. record --meta) also accesses it (translating each - to a _), while on 1.4 (. record -meta) works and (. record --meta) doesn't.

Actually, looking at line 883 of Compiler.java, it looks like this may be related to the (. foo -property) syntax ported from CLJS, and indeed (. record ---meta) works, I guess by reducing to an "old style" (. record --meta). So that clears up why --meta fails: it's looking for __meta. I'm still not clear on why (. record -meta) works, though.

So it looks like the - prefix for properties is not 100% backwards-compatible like it seemed to be. Is this an issue we need to fix, or is the recommendation simply to never have fields that start with - or _?



 Comments   
Comment by Fogus [ 09/Feb/12 2:33 PM ]

Is this a general problem with fields starting with _ or just fields named __meta as in (defrecord [__meta] ...)

Comment by Alan Malloy [ 09/Feb/12 3:01 PM ]

It's a general issue. (defrecord [__meta]) actually breaks immediately, because the record mechanism itself generates a field named __meta, but any field named with a - or _ prefix has this issue.

user=> (defrecord Foo [-blah])
user.Foo
user=> (.-blah (Foo. 1))
IllegalArgumentException No matching field found: blah for class user.Foo clojure.lang.Reflector.getInstanceField (Reflector.java:289)

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

The . syntax is interop and should always be used with Java-style field names. If other things work, that's undocumented and subject to change. You can access fields by using either . (which can also refer to methods) or .- (for field access only if there is ambiguity) - also see https://clojure.org/reference/java_interop.

The munging style for internal record field names should be considered an implementation detail, although maybe one it's useful to know about.

For the internal _meta example, which will become a Java field named "_meta":

;; Things that should (and do) work for interop:
(.__meta foo)
(. foo __meta)
(.-__meta foo)
(. foo -__meta)

Anything with --meta is undefined and unsupported.

For a field that starts with -, the following should be expected to work for the munged name ("_blah"):

(defrecord R [-blah])
(def r (->R 10))

(._blah r)
(. r _blah)
(.-_blah r)
(. r -_blah)

Anything Java interop stuff with -blah is undefined and unsupported.





[CLJ-190] enhance with-open to be extensible with a new close multimethod Created: 13/Sep/09  Updated: 23/Feb/17  Resolved: 23/Feb/17

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: io


 Description   

Discussion: http://groups.google.com/group/clojure/browse_thread/thread/8e4e56f6fc65cc8e/618a893a5b2a5410

Currently, with-open calls .close when it's finished. I'd like it to have a (defmulti close type) so it's behavior is extensible. A standard method could be defined for java.io.Closeable and a :default method with no type hint. I've come across a few cases where some external library defines what is essentially a close method but names it shutdown or disable, etc., and adding my own "defmethod close" would be much easier than rewriting with-open. This would also allow people to eliminate reflection for classes like sql Connection that were created before Closeable.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 4:30 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/190
Attachments:
clojure-190-with-open.patch - https://www.assembla.com/spaces/clojure/documents/ca27R6Ojur3PQ0eJe5afGb/download/ca27R6Ojur3PQ0eJe5afGb

Comment by Assembla Importer [ 24/Aug/10 4:30 AM ]

mikehinchey said: [file:ca27R6Ojur3PQ0eJe5afGb]: fix adds close method and tests

Comment by Assembla Importer [ 24/Aug/10 4:30 AM ]

mikehinchey said: Note, I only defined methods for :default (reflection of .close) and Closeable, not sql or the numerous other classes in java that should be Closeable but are not. Maybe clojure.contrib.sql and other such libraries should define related close methods.

Comment by Assembla Importer [ 24/Aug/10 4:30 AM ]

richhickey said: I want to hold off on this until scopes are in

Comment by Tassilo Horn [ 23/Dec/11 6:50 AM ]

Probably better implemented using a protocol. See http://dev.clojure.org/jira/browse/CLJ-308

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

Superseded by CLJ-308





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

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

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


 Description   

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

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

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

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

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

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

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

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

Proposed solution: Implement this spec:

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

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

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

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

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



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

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

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

Why?

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

Because spec provides the tools to do it already.

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

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

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

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

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

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

Some factors I try to think about:

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

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

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

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





[CLJ-200] Extend cond to support inline let, much like for Created: 18/Oct/09  Updated: 23/Feb/17  Resolved: 01/Jul/16

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Mark Engelberg
Resolution: Declined Votes: 11
Labels: None

Attachments: File clj-200-cond-let-clauses-fixed-test-v2.diff     Text File clj-200-cond-let-clauses-fixed-test-v2-patch.txt    
Patch: Code and Test

 Description   

I find it occasionally very useful to do a few tests in a cond, then introduce some new symbols (for both clarity and efficiency) that can be referenced in later tests (or matching expressions). This parallels similar functionality inside the for macro, where the :let keyword is matched against a vector of symbol bindings and forms an implicit let around the remainder of the comprehension.

I'll be adding a patch for this shortly.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/200

Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

hlship said: Trickier than I thought because cond is really wired into other fundamentals, like let.

Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

cgrand said: Howard, what do you think of http://gist.github.com/432712 ?

Comment by Mark Engelberg [ 23/Nov/12 2:33 AM ]

Patch cond-let-clauses.diff on 23/Nov/12 adds inline :let clauses to cond, implementing CLJ-200. The code is based off of code by cgrand, with some tweaks so the implementation only relies on constructs defined earlier in core.clj, since when cond is defined, things aren't yet fully bootstrapped. Also added a test to control.clj.

Comment by Christophe Grand [ 23/Nov/12 3:06 AM ]

Some comments: the docstring is missing, I believe you don't have to modify the original cond (except the docstring maybe), just redefine it later on once most of the language is defined – a bit like what is done for let for example.

There is still the unlikely eventuality that some code uses :let as :else. What about shipping a cond which complains on keywords (in test position) other than :else?

Comment by Mark Engelberg [ 23/Nov/12 3:47 AM ]

cond-let-clauses-with-docstring.diff contains the same patches as cond-let-clauses, but includes the original docstring for cond along with an additional sentence about the :let bindings.

Comment by Mark Engelberg [ 23/Nov/12 3:54 AM ]

Cgrand, I did see your example of redefining cond after most of the language is defined, but since I was able to figure out how to do it in the proper place, that makes the :let bindings available for users of cond downstream and avoids any unforeseen complications that might come from rebinding.

As for your other point, I think it is highly improbable that someone would have used :let in the :else position. However I can imagine someone intentionally using something like :true or :default. I think the idea of warning for other keywords is actually more likely to cause complications than the unlikely problem it is meant to solve.

I did resubmit the patch with the docstring restored. Thanks for pointing out that problem. I'm excited about this patch – I use :let bindings within the cond in my own code all the time. Thanks again for the blog post that started me on that path.

Comment by Christophe Grand [ 23/Nov/12 4:13 AM ]

True, it's :unlikely for :let to happen.
However once :let is officially blessed, it may be better to provision for future other "special" keywords and thus to warn on "unsupported" keywords. Plus it will help out-of-order typists (like myself) to catch earlier a :elt instead of a :let
This is only my point of view. Thanks for trying to get :let in cond supported.

Comment by Andy Fingerhut [ 29/Nov/12 8:46 PM ]

Mark, could you remove the obsolete earlier patch now that you have added the one with the doc string? Instructions for removing patches are under the heading "Removing Patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Comment by Mark Engelberg [ 29/Nov/12 10:50 PM ]

Done.

Comment by Andy Fingerhut [ 30/Nov/12 1:24 AM ]

I haven't figured out what is going wrong yet. I can apply the patch cond-let-clauses-with-docstring.diff to the latest Clojure master just fine. I can do "ant jar" and it will build a jar. When I do "ant", it fails with the new test for cond with :let, throwing a StackOverflowException. I can enter that same form into the REPL and it evaluates just as the test says it should. I can comment out that new test and all of the rest pass. But the new test doesn't pass when inside of the control.clj file. Anyone know why?

Comment by Christophe Grand [ 30/Nov/12 4:54 AM ]

It's because of the brutal replacement performed by test/are: the placeholders for this are form are x and y but in Mark's test there are used as local names and are tries to substitute them recursively...
If one changes the local names to a and b for example it works.

Comment by Mark Engelberg [ 02/Dec/12 8:20 AM ]

cond-let-clauses-fixed-test.diff on 02/Dec/12 contains the same patch, but with the x,y locals in the test case changed to a,b so that it works properly in the are clause which uses x and y.

Comment by Mark Engelberg [ 02/Dec/12 8:27 AM ]

On Windows, I can't get Clojure's test suite task to work, either via ant or maven, which has made it difficult for me to verify the part of the patch that applies to the test suite works as expected; I had tested it as best I could in the REPL, using a version of Clojure built with the patch applied, but using this process, I missed the subtle interaction between are and the locals in the test case. Sorry about that. If someone can double-check that the test suite task now works with the newest patch, that would be great, and then I'll go ahead and remove the obsoleted patch. Thanks.

Comment by Andy Fingerhut [ 02/Dec/12 6:29 PM ]

clj-200-cond-let-clauses-fixed-test-v2-patch.txt dated Dec 2 2012 is identical to Mark Engelberg's cond-let-clauses-fixed-test.diff of the same date, except it applies cleanly to the latest Clojure master.

I've verified that it compiles and passes all tests with latest Clojure master as of this date.

Mark, I've made sure to keep your name in the patch, since you wrote it. You should be able to remove your two attachments now, so the screener won't be confused which patch should be examined.

Comment by Andy Fingerhut [ 02/Dec/12 6:31 PM ]

Mark, besides general issues with Windows not being used much (or maybe not at all?) by Clojure developers, there is the issue right now filed as CLJ-1076 that not all tests pass when run on Windows due to CR-LF line ending differences that cause several Clojure tests to fail, regardless of whether you use ant or maven to run them.

Comment by Andy Fingerhut [ 22/Oct/13 8:01 PM ]

clj-200-cond-let-clauses-fixed-test-v2.diff is identical to earlier patch clj-200-cond-let-clauses-fixed-test-v2-patch.txt, except it removes unnecessarily trailing whitespace that causes warnings when applying the patch, and with .diff suffix for easier diff-style viewing in some editors.

Comment by Mark Engelberg [ 01/Jul/16 7:09 PM ]

It was just brought to my attention that this issue was declined today.

FWIW, earlier today I wrote up a Rationale about this issue as part of my effort to address it temporarily at the library level. I hope you will take a look:

https://github.com/Engelberg/better-cond#rationale

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

This ticket was resolved, but not closed. Just moving this ticket into the right jira state.





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

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

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

Ubuntu 16.04 x86_64, OpenJDK 8



 Description   

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

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

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

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

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

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

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

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

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



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

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

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

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

So, your ns declaration should instead be:

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

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





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

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

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


 Description   

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

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

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

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






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

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

Type: Enhancement Priority: Minor
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: try-catch
Environment:

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

Mac OSX 10.9.4


Attachments: File ex_info_arity.diff    
Patch: Code
Approval: Triaged

 Description   

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

(ex-info "the exception message")

That created a ExceptionInfo instance carries empty data.

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

The patch is attached.



 Comments   
Comment by Alex Miller [ 10/Feb/17 8:46 AM ]

Why "(. clojure.lang.PersistentArrayMap EMPTY)" ? Why not just {}?

Comment by Leon Grapenthin [ 14/Feb/17 1:43 PM ]

I always thought the lack of a one-arity was intentional design to make users use the map argument. Why do you want to throw ExceptionInfos with no data?

Comment by dennis zhuang [ 14/Feb/17 9:53 PM ]

@Alex I forgot why i used EMPTY map here, maybe influenced by the code https://github.com/clojure/clojure/blob/7aad2f7dbb3a66019e5cef3726d52d721e9c60df/src/clj/clojure/core.clj#L4336

@Leon For example, throw an exception when arguments error:

(when-not (integer? c)
(throw (ex-info "Expect number for c."))

We don't need data here.

Comment by Nicola Mometto [ 15/Feb/17 4:27 AM ]

Then why not just throw an `(Exception. "expect number for c")`? I don't see the added value in throwing exinfos w/o data vs just throwing an Exception

Comment by dennis zhuang [ 21/Feb/17 9:13 PM ]

Indeed, it was just a technical decision that we chose to use ex-info for throwing exceptions.





[CLJ-2103] s/coll-of and s/every take unnecessary long to generate if :into not provided Created: 28/Jan/17  Updated: 17/Feb/17

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

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

alpha14



 Description   
(s/def ::bar (s/coll-of number? :kind vector?))

(defn foo [a]
  (reduce + 0 a))

(s/fdef foo
  :args (s/cat :a ::bar)
  :ret number?)

(first (stest/check `foo))

Doesn't terminate in a reasonable amount of time.
One observes that changing :gen-max doesn't affect computation time.

One observes that it can be fixed by adding :into [] to the s/coll-of spec.

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

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

Assumedly the vector? generates quite large vectors at a certain point which significantly slows down the generation.

The responsible code is in gen* of every-impl

(gen/bind
 (cond
  gen-into (gen/return (empty gen-into))
  kind (gen/fmap #(if (empty? %) % (empty %))
                 (gensub kind overrides path rmap form))  ;; creating and mapping gen of :kind
 :else (gen/return []))
 (fn [init]
    ....


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

I see two approaches to improve this behavior:

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





[CLJ-1138] data-reader returning nil causes exception Created: 22/Dec/12  Updated: 15/Feb/17

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

Type: Defect Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: reader
Environment:

clojure 1.5 beta2, Mac OS X 10.8.2, java version "1.6.0_37"


Attachments: Text File 0001-CLJ-1139-allow-nil-in-data-reader.patch     Text File clj-1139-2.patch    
Patch: Code and Test
Approval: Triaged

 Description   

If a data-reader returns nil, the reader throws java.lang.RuntimeException: No dispatch macro... The error message implies that there is no dispatch macro for whatever the first character of the tag happens to be.

Here's a simple example:

user=> (binding [*data-readers* {'f/ignore (constantly nil)}] 
         (read-string "#f/ignore 42 10"))
RuntimeException No dispatch macro for: f  clojure.lang.Util.runtimeException (Util.java:219)

The original reader code did not distinguish between the absence of a data-reader and a returned value of nil from the appropriate data-reader. It therefore got confused and tried to find a dispatch macro, sending it further down the incorrect code path, ultimately yielding a misleading error message.

The original documentation did not distinguish nil as an illegal value. Clearly this bug was an oversight in the original data-reader code, not an intentional feature.

The patch uses a sentinel value to distinguish the missing data-reader case from the nil returned value case.

Patch: clj-1139-2.patch



 Comments   
Comment by Steve Miner [ 22/Dec/12 9:43 AM ]

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch allows a data-reader to return nil instead of throwing. Does sanity check that possible tag or record isJavaIdentifierStart(). Gives better error message for special characters that might actually be dispatch macros (rather than assuming it's a tagged literal).

Comment by Steve Miner [ 22/Dec/12 10:06 AM ]

clj-1138-data-reader-return-nil-for-no-op.patch allows a data-reader returning nil to be treated as a no-op by the reader (like #_). nil is not normally a useful value (actually it causes an exception in Clojure 1.4 through 1.5 beta2) for a data-reader to return. With this patch, one could get something like a conditional feature reader using data-readers.

Comment by Steve Miner [ 22/Dec/12 10:26 AM ]

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch is the first patch to consider. It merely allows nil as a value from a data-reader and returns nil as the final value. I think it does what was originally intended for dispatch macros, and gives a better error message in many cases (mostly typos).

The second patch, clj-1138-data-reader-return-nil-for-no-op.patch, depends on the other being applied first. It takes an extra step to treat a nil value returned from a data-reader as a no-op for the reader (like #_).

Comment by Steve Miner [ 23/Dec/12 11:52 AM ]

It turns out that you can work around the original problem by having your data-reader return '(quote nil) instead of plain nil. That expression conveniently evaluates to nil so you can get a nil if necessary. This also works after applying the patches so there's still a way to return nil if you really want it.

(binding [*data-readers* {'x/nil (constantly '(quote nil))}] (read-string "#x/nil 42"))
;=> (quote nil)

Comment by Andy Fingerhut [ 07/Feb/13 9:20 AM ]

Patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch dated Dec 22 2012 still applies cleanly to latest master if you use the following command:

% git am --keep-cr -s --ignore-whitespace < clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch

Without the --ignore-whitespace option, the patch fails only because some whitespace was changed in Clojure master recently.

Comment by Andy Fingerhut [ 13/Feb/13 11:24 AM ]

OK, now with latest master (1.5.0-RC15 at this time), patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch no longer applies cleanly, not even using --ignore-whitespace in the 'git am' command given above. Steve, if you could see what needs to be updated, that would be great. Using the patch command as suggested in the "Updating stale patches" section of http://dev.clojure.org/display/design/JIRA+workflow wasn't enough, so it should probably be carefully examined by hand to see what needs updating.

Comment by Steve Miner [ 14/Feb/13 12:21 PM ]

I removed my patches. Things have changes recently with the LispReader and new EdnReader.

Comment by Alex Miller [ 15/Feb/17 9:18 AM ]

Fixed whitespace warning and updated patch so it applies, no semantic changes, attribution retained in clj-1139-2.patch.

Comment by Alex Miller [ 15/Feb/17 9:27 AM ]

Ticket needs better description of problem and approach taken in the patch.

Comment by Steve Miner [ 15/Feb/17 10:09 AM ]

If the problem isn't clear, I would ask why would a nil return value be treated specially for a data-reader? And if it is considered illegal by design, does this error message enlighten the user?

I could not find any documented restriction at the time the bug was filed and I still can't find any today. So it seems like a simple bug to me. The data-reader should be allowed to return nil, and the Clojure reader should process the nil as usual. My work-around was to return (quote nil) which gave the intended behavior without triggering the bug.

Comment by Alex Miller [ 15/Feb/17 10:55 AM ]

Would appreciate more updates to the description. My question would be whether invoking a data reader function should ever return nil. Is there a good use case to need this? It seems you are reading the description of a non-nil tagged value with the reader and thus getting back nil is confusing. That's not possibly round-trippable and thus seems asymmetric.

Comment by Steve Miner [ 15/Feb/17 1:34 PM ]

Nulla poena sine lege or basically unless you say it's illegal, I should be able to do it. My trivial example is #C NULL which seems like an obvious nil to me.

Looking back on this issue, I can see that most people think of tagged literals as a way of encoding foreign values in Clojure literals. If you only care about an extensible data notation, who needs another way of writing nil? That's a fair question.

I wanted to use data-readers as somewhat circumscribed reader macros (as used in Common Lisp). I discovered this bug while I was doing something platform specific (long before reader conditionals were implemented). In my situation, it was convenient to return nil on "other" platforms.

Many usages of data-readers are not bijective. For example, #infix (3 + 4) interpreted as constant 7 is likewise not round-trippable. Unless you're Dan Friedman or Wil Byrd, round-tripping is a tough requirement.

I will try to update my description with a bit more context, but I don't want to distract anyone from the obvious bug (and bad error message) with my unorthodox usage.

Comment by Steve Miner [ 15/Feb/17 1:45 PM ]

By the way, this bug is CLJ-1138, but the proposed patch says "1139" which might confuse some busy reviewers.

Comment by Steve Miner [ 15/Feb/17 2:13 PM ]

I tested the patch and it worked well for me with the current master. I would suggest adding another test to confirm that the edn/read-string works correctly as well. Here's what I used. This also tests that overriding the default readers works. Please feel free to take the test if you want it.

(deftest clj-1138-uuid-override
  (is (nil? (binding [*data-readers* {'uuid (constantly nil)}]
              (read-string "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))
  (is (nil? (edn/read-string {:readers {'uuid (constantly nil)}}
                             "#uuid \"550e8400-e29b-41d4-a716-446655440000\""))))




[CLJ-2112] Add specs for spec forms Created: 15/Feb/17  Updated: 15/Feb/17

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

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

Attachments: Text File spec-forms.patch    
Patch: Code
Approval: Incomplete

 Description   

It would be useful to have specs that described spec forms, such that it was possible to go from a spec form like (s/keys :req [::a ::b] :opt [::c]) to a conformed version that allowed you to grab the parts without parsing the s-expression. This can be done by creating specs, thus allowing:

user=> (require '[clojure.spec :as s] '[clojure.spec.specs])
user=> (s/def ::aspec (s/keys :req [::a ::b] :opt [::c]))
user=> (def aspec-data (s/conform :clojure.spec.specs/spec (s/form ::aspec)))
user=> (pr aspec-data)
[:form {:s clojure.spec/keys, 
        :args {:req [[:key :clojure.spec.specs/a] [:key :clojure.spec.specs/b]], 
               :opt [:clojure.spec.specs/c]}}]
user=> (map val (-> aspec-data val :args :req))
(:clojure.spec.specs/a :clojure.spec.specs/b)

Patch: spec-forms.patch (a work in progress)






[CLJ-2040] Allow runtime modification of REPL exception handling Created: 11/Oct/16  Updated: 15/Feb/17

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

Type: Enhancement Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File CLJ-2040-dynamic-repl-exceptions.patch    
Patch: Code
Approval: Triaged

 Description   

Problem Statement

Clojure's REPL is capable of paramterizing almost every aspect of its functionality, including how uncaught exceptions are printed. In the current implementation, these customization hooks are passed in as arguments and closed over, meaning that they cannot be changed once the REPL is started.

Many development tools want to override how the REPL handles uncaught errors. Examples of useful customizations include (but are not limited to):

  • Formatted exception messages (including whitespace and ANSI coloring)
  • Alternative representations for certain types of exceptions (e.g, Spec errors)
  • Dropping into a graphical interaction mode to better inspect ex-data.

Currently, this type of customization must be applied before a REPL is started, meaning that changing how a REPL displays errors requires support from (or plugins to) a third-party tool such as Boot or Leiningen.

Alternatives


1. Take no action.

Third-party tool support is required to create customized exception handling in the REPL. Tools have different techniques for doing this:

  • nREPL can intercept the exception on the wire and passes it through middleware
  • Leiningen plugins alter the root binding of clojure.main/repl-caught.
  • Boot allows users to build a task to invoke clojure.main/repl with the desired arguments.

Users will continue to select one of these according to their tooling preferences.

Benefits:
1. No effort or changes to the existing code.

Tradeoffs:
1. Tools will continue to implement their own diverse, sometimes hacky techniques for printing custom exceptions.
2. Any library intended to provide alternative exception handling will be tied to a specific launcher tool.

2. Make the REPL exception handler dynamically rebindable

If the REPL exception handler were a dynamic, thread-local var, users and libraries could change the behavior of the currently running REPL.

Benefits:
1. Users and libraries can freely override how exceptions are printed, regardless of how Clojure was launched.
2. Fully backwards compatible with existing tools.

Tradeoffs:
1. It will be possible for library authors to provide "bad" or poorly reasoned error printers. This is still possible with launch tools, but the barrier of entry is even lower with libraries.

The attached patch implements this option.

3. Encourage users to start new REPLs instead

In many Clojure environments, it's possible to explicitly launch a REPL from within another REPL. This sub-REPL could have the desired :caught hook.

Benefits:
1. No effort or changes to the existing code.
2. "Functionally pure", and in alignment with the evident design of the current REPL.

Tradeoffs:
1. There is a non-trivial subset of Clojure developers who do not know exactly how REPLs work. They are likely to be confused or subject to increased cognitive load. Insofar as this set of beginner/intermediate developers are precisely who enhanced error messages are meant to help in the first place, this solution is counterproductive.
2. For better or for worse, many existing and widely used tools do not support this. This does not work at all in nREPL, for example. However, even the simplest command-line REPLs behavior would change for the worse; sending a EOF (accidentally or otherwise) would always kill the sub-REPL with no feedback as to what just happened.



 Comments   
Comment by Alex Miller [ 15/Feb/17 9:39 AM ]

On the repl-caught var, it would be good to mention the signature of the handler, namely that it takes an exception, is expected to print or otherwise handle the exception, and that its return will be ignored.

Rather than the changes in repl, what if you added a dynamic-repl-caught and change that to be the default caught handler in repl? Or even just changed repl-caught itself.





[CLJ-2111] s/every :kind does not work as intended Created: 14/Feb/17  Updated: 14/Feb/17

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

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

Approval: Vetted

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

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

Expected: true



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

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

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

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





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

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

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

Oracle Java 1.8.0_112



 Description   

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

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

Expected: {:v 3}
Actual: nil



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

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

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

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

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

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

Ok. But how about this?:

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

Is it OK?

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

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

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

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

Agreed with Andy, declining





[CLJ-2108] Loading core specs affects startup time Created: 13/Feb/17  Updated: 13/Feb/17

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

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

Approval: Vetted

 Description   

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



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

Locally,
alpha14 takes 1.02s

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

Master with this line commented out takes 0.92s

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




[CLJ-2109] Protocol methods not instrumented Created: 13/Feb/17  Updated: 13/Feb/17

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

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: spec
Environment:

Clojure 1.9.0-alpha14


Approval: Triaged

 Description   

Spec instrument does not work on protocol methods. Invalid arguments will be accepted silently with no error. Protocol vars are included in the return value of (instrument).

Steps to reproduce

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

(defprotocol P
  (method [this arg]))

(defrecord R []
  P
  (method [this arg]
    (str "R.method called with " (pr-str arg))))

(s/fdef method
  :args (s/cat :this any?
               :arg number?))

(defn wrapped [this arg]
  (method this arg))

(s/fdef wrapped
  :args (s/cat :this any?
               :arg number?))

(test/instrument)

(println (method (->R) "not a number"))

(println (wrapped (->R) "not a number"))

This code produces the output:

R.method called with "not a number"
clojure.lang.ExceptionInfo: Call to #'user/wrapped did not conform to spec:
In: [1] val: "not a number" fails at: [:args :arg] predicate: number?
...

Possible resolutions

1. Add support to instrument for protocol methods
2. Document that instrument does not work on protocol methods, do not return protocol method Vars from (instrument), throw exception if protocol method Vars are included in the symbols passed to (instrument syms)

See also

CLJ-1941 describes a different case where instrument does not work. This issue was identified in a comment.

Workarounds

This issue can be avoided by wrapping protocol methods in normal functions and spec'ing the functions. This is already common practice.






[CLJ-1776] Test that collections are valid statements Created: 08/Jul/15  Updated: 09/Feb/17

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Michael Blume
Resolution: Unresolved Votes: 0
Labels: compiler, test

Attachments: Text File clj-1776-v1.patch     Text File CLJ-1776-v2.patch    
Patch: Code and Test

 Description   

It's possible to break the compiler such that vectors are emitted incorrectly when they're in statement position (I accidentally did this). This doesn't break any part of the Clojure test suite, but does break valid Clojure code (for me it hit taoensso's encore). Add tests to the test suite so defects of this kind are caught.



 Comments   
Comment by Daniel Compton [ 09/Feb/17 8:34 PM ]

Is there a compiler change required as well here?

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

I think he is saying that the compiler is fine but that if it weren't, no test would tell you that.





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

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

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


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

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

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

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

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

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



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

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

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

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

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

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

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

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

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

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

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

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





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

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

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


 Description   

This explanation is clear:

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

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

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

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

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

I would expect the same predicate: string? explanation.

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



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

Explanation of a workaround from Slack from @hiredman:

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

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

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

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

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

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

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





[CLJ-2105] incorrect spec conform when an optional (?) is inside of a one or more (+) Created: 03/Feb/17  Updated: 03/Feb/17

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

Type: Defect Priority: Major
Reporter: Anthony D'Ambrosio Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

1.9.0-alpha14


Approval: Vetted

 Description   
(s/def ::thing (s/cat :a (s/? string?) :b (s/+ number?))) 
(s/def ::seq-of (s/+ ::thing))

(s/conform ::seq-of (list "foo" 1 "bar" 2 3 "qux" 4)) 
;=> [{:a "foo", :b [1]} [{:a "bar", :b [2 3]} {:a "qux", :b [4]}]]

;EXPECTED 
;=> [{:a "foo", :b [1]} {:a "bar", :b [2 3]} {:a "qux", :b [4]}]

;; ONLY 2nd thing matters? 
(s/conform ::seq-of (list "foo" 1 2 "bar" 3)) 
;=> [{:a "foo", :b [1 2]} {:a "bar", :b [2]}]

;; NO OPTIONAL 
(s/def ::thing (s/cat :a string? :b (s/+ number?))) 
(s/def ::seq-of (s/+ ::thing))
(s/conform ::seq-of (list "foo" 1 "bar" 2 3 "qux" 4)) 
;=> [{:a "foo", :b [1]} {:a "bar", :b [2 3]} {:a "qux", :b [4]}]

This also only shows up if there are 2+ numbers in the 2nd or later ::thing
AND the problem goes away if I make :a not optional...

Could be related to http://dev.clojure.org/jira/browse/CLJ-2003 ?






[CLJ-2080] clojure.spec/every-kv does not work correctly on vectors Created: 08/Dec/16  Updated: 31/Jan/17

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

Type: Defect Priority: Major
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: spec
Environment:

alpha 14


Attachments: Text File clj-2080-2.patch     Text File clj-2080-3.patch     Text File clj-2080.patch    
Patch: Code
Approval: Vetted

 Description   

The every-kv doc states "takes separate key and val preds and works on associative collections". Vectors return true for associative? but do not currently work:

user=> (s/conform (s/every-kv any? any?) [])
[]
user=> (s/conform (s/every-kv any? any?) [1 2 3])
:clojure.spec/invalid
user=> (s/conform (s/every-kv integer? string?) [])
[]
user=> (s/conform (s/every-kv integer? string?) ["x"])
:clojure.spec/invalid

Another similar problem:

(s/explain-data (s/every-kv int? int?) [{:a :b}])
UnsupportedOperationException nth not supported on this type: PersistentArrayMap  clojure.lang.RT.nthFrom (RT.java:903)

Cause: The combination of every-kv and every-impl assume that the collection passed to every-kv can provide a seq of map entries (which is the intention). In the explain case, the ::kfn created by every-kv is used to create a better path segment using the key rather than the element index. The kfn assumes that an element of the collection can call `(nth entry 0)` on the element. In the explain failure above, the map {:a :b} will throw when invoked with nth.

Proposed: Do the following to be clearer about the requirement that the coll elements are map entries:

  • Modify the docstring to say "seqs to map entries" rather than "is a map"
  • Modify the kfn to add a check that the element is an entry and if so, use it's key. If not, use the index (of the element itself). In this case, when passed an entry it will report with the actual key but when passed something that's not an entry, it will report the collection based index of the non-entry.

Patch: clj-2080-3.patch



 Comments   
Comment by Alex Miller [ 09/Dec/16 8:35 AM ]

At the moment, I'm inclined to say the doc in every-kv should be tightened to say "map" instead of "associative collection" but will check with Rich.





[CLJ-2056] Efficient shortcut for (first (filter pred coll)) idiom Created: 11/Nov/16  Updated: 30/Jan/17

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

Type: Enhancement Priority: Major
Reporter: Nikita Prokopov Assignee: Unassigned
Resolution: Unresolved Votes: 27
Labels: None

Attachments: Text File clj-2056-clojure-core-seek-2.patch    
Patch: Code and Test
Approval: Prescreened

 Description   

It's a common task to look up for an item in a collection based on predicate. Currently Clojure has no direct support for that in clojure.core. Instead, our options are:

1. (first (filter pred coll)) will create intermediate lazy sequence and might evaluate pred up to 31 extra times in case of chunked sequence

2. (some #(when (pred %) %) coll) will short-circuit on first match, but won't catch false value in something like (some #(when false? %) [true false true])

Additionally, both of these workarounds a) obscure the purpose of the code, and b) do not handle custom not-found values.

Attached is a patch that makes use of efficiency of reduce-able collections, handles edge cases like looking for false? or nil?, and supports optional not-found value.

Examples:

(seek odd? (range)) => 1
(seek pos? [-1 1]) => 1
(seek pos? [-1 -2] ::not-found) => ::not-found
(seek nil? [1 2 nil 3] ::not-found) => nil

Patch: clj-2056-clojure-core-seek-2.patch

Prescreening notes: I think the general approach is good. Is it necessary to support nil? and false? preds? Or would a transduce formulation like the one in comments be sufficient.



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

Just as an interesting aside, the new halt-when transducer could actually be used to create something like this too (if you set aside the desire to support nil? and false? preds).

(transduce (comp (filter pred) (halt-when any?)) identity nil coll)

Patch has some trailing whitespace in the test code - could you clean that up?

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Attaching patch with trailing whitespace cleaned

Comment by Nikita Prokopov [ 12/Nov/16 3:46 AM ]

Thanks Alex! Attached new patch with whitespace cleaned

Comment by Moritz Heidkamp [ 30/Jan/17 12:34 PM ]

I had a similar train of thought today and arrived at the idea of adding transducer support to first, e.g. (first (filter pred) coll). This could potentially be as efficient as the special-purpose seek function proposed above but would of course not support the not-found value. However, it seems like a natural extension to first and could be useful for other purposes, too.





Generated at Mon Mar 27 05:59:35 CDT 2017 using JIRA 4.4#649-r158309.