<< Back to previous view

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

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

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

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

 Description   

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

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

Depends on CLJ-1488



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

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

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





[CLJ-1488] Implement Named over Vars Created: 01/Aug/14  Updated: 11/Aug/14

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

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

Attachments: Text File 0001-Implement-clojure.lang.Named-over-Vars.patch    
Patch: Code
Approval: Triaged

 Description   

Vars, while a general reference structure, are used to implement bindings and have special reader and printer notation reflecting this reality. Unlike Keywords and Symbols which share the "namespace/name" notation of Vars, Vars do not implement the clojure.lang.Named interface while they print as if they were Named.

The attached patch implements Named over Vars.

Example:

user=> (name :clojure.core/conj)
"conj"
user=> (namespace :clojure.core/conj)
"clojure.core"
user=> (name 'clojure.core/conj)
"conj"
user=> (namespace 'clojure.core/conj)
"clojure.core"
user=> (name #'clojure.core/conj)
"conj"
user=> (namespace #'clojure.core/conj)
"clojure.core"
user=> (with-local-vars [x 1] (name x))
"--unnamed--"
user=> (with-local-vars [x 1] (namespace x))
nil
user=> (with-local-vars [x 1] (println x))
#<Var: --unnamed-->

This is useful for applications such as the CinC project where Vars are often taken directly as values in which context they would ideally be interchangeable with the Symbols the bound values of which they represent.



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

With this patch calling `name` on a unnamed Var will cause a NPE, I don't think this is desiderable.

Comment by Reid McKenzie [ 02/Aug/14 1:39 PM ]

I agree, however this behavior seems to be standard in Core.

Clojure 1.6.0
user=> (name nil)
NullPointerException clojure.core/name (core.clj:1518)
user=> (namespace nil)
NullPointerException clojure.core/namespace (core.clj:1526)

I'm also not convinced that the "name" or "namespace" of an unbound var is meaningful, in which case a NPE is probably acceptable.

Comment by Nicola Mometto [ 02/Aug/14 1:45 PM ]

I was not talking about unbound Vars, but about anonymous Vars, I'm assuming you miswrote.

I'd agree with you that throwing an exception could be a reasonable behaviour, except I can test for nil before calling name on it while there's no way to test whether a var is named or not, except trying to access directly the .name field which is excatly what this ticket is for.

Comment by Nicola Mometto [ 02/Aug/14 2:27 PM ]

Me and Reid have been talking about this issue over IRC, here's what's come up:

  • Vars can be either unnamed (as are Vars returned by with-local-vars) or contain both a namespace and a name part( that's the case for interned Vars)
  • there's currently no way to test for the "internedness" of a Var, so accessing either the .name or the .namespace field of the Var testing for nil is the only way to do it currently

given the above, the current patch seems unsatisfactory, here some proposed solutions:

  • make Var Named, make namespace return nil for an unnamed Var and name return "--unnamed--"
  • keep Var not implementing Named, add a "var-symbol" function returning either a namespaced symbol matching the ns+name of the Var or nil for an unnamed Var

Personally, I'd rather have the second solution implemented as I don't feel Var should be Named given that they can be unnamed and that strikes me as a contradicion

Comment by Reid McKenzie [ 02/Aug/14 3:16 PM ]

Added patches explicitly handling the unnamed var cases.

Comment by Reid McKenzie [ 02/Aug/14 3:33 PM ]

Squashed all patches into a single diff and updated attachments.





[CLJ-1483] Clarify the usage of replace(-first) with a function Created: 27/Jul/14  Updated: 29/Jul/14

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

Type: Enhancement Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, string

Attachments: Text File 0001-Clarify-the-usage-of-replace-first-with-pattern-func.patch    
Patch: Code
Approval: Triaged

 Description   

The documentation of replace and replace-first didn't feature any example usage of the pattern + function combo so I've added one.






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

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

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

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

 Description   

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






[CLJ-1481] Typo in type-reflect's docstring Created: 27/Jul/14  Updated: 29/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Fix-a-typo.patch    
Patch: Code
Approval: Triaged

 Description   

membrer -> member






[CLJ-1480] Incorrect param name reference in defmulti's docstring Created: 27/Jul/14  Updated: 29/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Fix-param-name-reference-in-defmulti-s-docstring.patch    
Patch: Code
Approval: Triaged

 Description   

attribute-map should actually be attr-map






[CLJ-1479] Typo in filterv example Created: 27/Jul/14  Updated: 29/Jul/14

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

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

Attachments: Text File 0001-Fix-a-typo.patch    
Patch: Code
Approval: Triaged

 Description   

filter -> filterv






[CLJ-1478] Doc typo Created: 27/Jul/14  Updated: 29/Jul/14

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

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

Attachments: Text File 0001-Fix-a-typo.patch    
Patch: Code
Approval: Triaged

 Description   

Another small typo fix.






[CLJ-1477] Fixed a typo Created: 27/Jul/14  Updated: 29/Jul/14

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

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

Attachments: Text File 0001-Fix-a-typo.patch    
Patch: Code
Approval: Triaged

 Description   

Just a simple typo fix - "directy" -> "directly".






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

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

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

Attachments: Text File kwinvoke.patch    
Patch: Code

 Description   

KeywordInvoke callsites exist to fastpath lookups into records, but involve a lot of callsite machinery. Records don't have any fastpath for namespaced keyword accesses. This patch just falls back to an InvokeExpr when the keyword access is namespaced.

(:ns/kw foo)






[CLJ-1464] Incorrectly named parameter to fold function in reducers.clj Created: 12/Jul/14  Updated: 14/Jul/14

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

Type: Defect Priority: Trivial
Reporter: Jason Jackson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Rename-a-function-parameter-to-reflect-the-fold-func.patch    
Patch: Code
Approval: Triaged

 Description   

https://github.com/clojure/clojure/blob/master/src/clj/clojure/core/reducers.clj#L95
The 2-arity fold accepts reducef as parameter and then uses it as a combinef.
Instead it should accept combinef as parameter and then use it as a reducef, as every combine fn (monoid) is a reduce fn, but not every reduce fn is a combine fn (it's not associative).



 Comments   
Comment by Jason Jackson [ 12/Jul/14 2:58 PM ]

this is my first patch for clojure please double check everything. CA is done.

Comment by Andy Fingerhut [ 12/Jul/14 7:29 PM ]

Everything gets double checked whether it is your first patch or your 50th

At least as far as the format of the patch being correct, that it applies cleanly to the latest version of Clojure on the master branch, compiles and passes all tests cleanly, all of that is good.

Whether there is interest in taking your proposed change is to be decided by others. It may be some time before it is examined further.

Comment by Jozef Wagner [ 13/Jul/14 4:11 AM ]

This is not a defect. Quoting Rich, "If no combining fn is supplied, the reducing fn is used." (source)

There are three user supplied operations in fold: getting identity element (combinef :: -> T), reducing function (reducef :: T * E -> T) and combining function (combinef :: T * T -> T). For reduce, combining function is not needed but the rest two operations are needed. Thus reducing function (reducef) supplies identity element for reducers and only in folders the identity element is produced by combining function. In case where reducing fn is used for both reducing and combining, it must of course be associative and must handle objects of types T and E as a second argument.

Comment by Jason Jackson [ 14/Jul/14 12:14 AM ]

@Jozef I appreciate the feedback I still think my patch is correct, although I admit everyone's time is better spent not debating this small refactoring so feel free to close it.

One perspective to view my patch from is if we had protocols p/Monoid and p/Reducer. It's possible to reify any object that implements p/Monoid into p/Reducer, but not the other way around since not every p/Reducer is associative.

Comment by Jason Jackson [ 14/Jul/14 12:30 AM ]

From that perspective you could also say that in the 2-arity case the parameter "reducef" requires objects that implement both p/Monoid and p/Reducer, but in the 3-arity case the parameter "reducef" only requires p/Reducer

Comment by Jozef Wagner [ 14/Jul/14 1:38 AM ]

Note that reducef and combinef take different type of second argument, so not every combining function can be used as a reducing one. Your proposal is thus no better than the status quo. Consider following example:

(fold clojure.set/union conj [1 1 2 2 3 3 4 4])




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

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

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

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

 Description   

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



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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

3rd and final cut at this patch.





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

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

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

Any.



 Description   

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

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






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

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

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

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

 Description   

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

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

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

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

Add checks to prevent these errors.



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

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

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

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

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

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

Patches from non-contributors cannot be committed to Clojure.

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

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

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

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

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

Alex Miller — Tahdah!

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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





[CLJ-1417] clojure.java.io/input-stream has incorrect docstring Created: 07/May/14  Updated: 07/May/14

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

Type: Defect Priority: Trivial
Reporter: Dario Bertini Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, io, newbie

Approval: Triaged

 Description   

clojure/java/io.clj line 125

"Default implementations are defined for OutputStream, File, URI, URL,"

Should read

"Default implementations are defined for InputStream, File, URI, URL,"






[CLJ-1398] Update URLs in javadoc.clj Created: 02/Apr/14  Updated: 09/May/14

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

Type: Enhancement Priority: Trivial
Reporter: Eli Lindsey Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-update-apache-commons-javadoc-location.patch     Text File 0002-add-javadoc-lookup-for-guava-and-apache-commons-lang.patch     Text File 0003-add-javadoc-lookup-for-jdk8.patch    
Patch: Code

 Description   

Three minor fixes/enhancements to javadoc.clj:

0001 corrects the URLs for apache commons javadoc (the ones used in javadoc.clj no longer resolve).
0002 adds javadoc lookup for guava and apache commons lang3.
0003 adds javadoc lookup for jdk8.

(Note: contributor agreement is in the mail)



 Comments   
Comment by Andy Fingerhut [ 04/Apr/14 11:22 AM ]

Eli, thanks for the patches. It appears that you are not currently on the list of Clojure contributors here: http://clojure.org/contributing

It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Comment by Eli Lindsey [ 04/Apr/14 11:27 AM ]

> It is the policy of the Clojure team only to incorporate patches submitted by people who have signed and submitted a Clojure CA. Were you interested in doing that?

Yup! I mailed off the CA to Rich on Wednesday when this was filed; should be arriving shortly.

Comment by Eli Lindsey [ 09/May/14 8:18 PM ]

Just to note - Clojure CA went through and I'm listed on the contributors page now.





[CLJ-1394] Print multi method dispatch values in the exception messages. Created: 31/Mar/14  Updated: 01/Apr/14

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

Type: Enhancement Priority: Trivial
Reporter: Roman Scherer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: File pr-str-dispatch-value-safe.diff    
Patch: Code and Test

 Description   

The error messages of defmulti are at the moment not as helpful as they could be under certain circumstances. Calling this multi method with a lazy seq as it's dispatch argument raises the following exception:

(defmulti test-multi identity)
(defmethod test-multi 1 [arg] true)

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value: clojure.lang.LazySeq@3c6f1187

Sometimes it would be useful to actually see which values are in the lazy seq being dispatched on. A better error message could look like
this for example:

(test-multi (map identity [:x]))
;=> java.lang.IllegalArgumentException: No method in multimethod 'test-multi' for dispatch value (:x) of class clojure.lang.LazySeq

This patch addresses this issue by formatting the dispatch value via `pr-str` and printing the class before it is passed to the exception constructor. The same is also done for the methods in MultiFn.java that throw a dispatch value as part of their exception message.



 Comments   
Comment by Nicola Mometto [ 31/Mar/14 8:22 PM ]

What if the value is infinite lazy-seq?

Comment by Roman Scherer [ 01/Apr/14 2:50 AM ]

Nicola, I forgot those. But I think infinite sequences could be handled with:

(set! *print-length* 10)

I'll try it out and will update the patch later.

Any other edge cases in mind?

Comment by Roman Scherer [ 01/Apr/14 2:28 PM ]

After having read "Controlling run-away trains, onions, and exercise
bikes" [1] I now bind print-length and print-size when building
the error message. This helps when not being able to dispatch on this
for example:

(test-multi (let [x (atom 0)] (reset! x {:deeper x})))

However I'm not sure if this helps in the following case, where
dispatching would fail on an infinite seq.

(test-multi (iterate inc 0))

The above doesn't terminate in Clojure 1.6.0, nor does it when binding
print-length like the attached patch does.

[1] http://blog.n01se.net/blog-n01se-net-p-85.html





[CLJ-1371] divide(Object, Object) with (NaN, 0) does not return NaN Created: 07/Mar/14  Updated: 07/Mar/14

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

Type: Defect Priority: Trivial
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0)

ArithmeticException Divide by zero clojure.lang.Numbers.divide (Numbers.java:156)
user=> (/ Double/NaN 0)
Double/NaN



 Comments   
Comment by Alex Miller [ 07/Mar/14 7:50 AM ]

As per the Java Language Specification (http://docs.oracle.com/javase/specs/jls/se7/html/jls-4.html#jls-4.2.4),

"All numeric operations with NaN as an operand produce NaN as a result."

Comment by Yongqian Li [ 07/Mar/14 7:54 AM ]

But in the first example it produces an ArithmeticException.

Comment by Alex Miller [ 07/Mar/14 9:27 AM ]

Ah, I see the question now.

Here we are dividing a double by a long. In the first case, this is parsed as divide(Object, long) which then calls divide(Object, Object), which throws ArithmeticException if the second arg is 0 (regardless of the first arg).

In the second case it's parsed as divide(double, long) which just relies on Java to properly upcast the primitive long to a double to do the divide.

Note that making this call with 2 doubles does return NaN:

user=> (def x Double/NaN)
#'user/x
user=> (/ x 0.0)
NaN

or type hinting x to a double works as well:

user=> (def x Double/NaN)
#'user/x
user=> (/ ^double x 0.0)
NaN

I think one option to "fix" this behavior would be to add checks in divide(Object, Object) to check whether x is NaN and instead return NaN.





[CLJ-1357] It's a small typo in the gen-class doc-string Created: 17/Feb/14  Updated: 17/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File CLJ-1357-its-typo.patch    
Patch: Code
Approval: Triaged

 Description   

"It's" should be "its" (possessive) in "It's return value is ignored."






[CLJ-1337] defprotocol's docstring is out of date Created: 04/Feb/14  Updated: 04/Feb/14

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

Type: Defect Priority: Trivial
Reporter: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: Text File 0001-Update-defprotocol-s-docstring-to-remove-an-out-of-d.patch    
Patch: Code

 Description   

The docstring of defprotocol has an out-of-date description as follows:

defprotocol is dynamic, has no special compile-time effect, and defines no new types or classes.

Indeed, it used to have no compile-time effect. Today, however, it does generate a corresponding interface via gen-interface at compile time.

The patch just removes this description.






[CLJ-1329] Unused local variable in PersistentVector.cons() Created: 22/Jan/14  Updated: 02/Mar/14

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

Type: Enhancement Priority: Trivial
Reporter: Smit Shah Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections

Attachments: Text File fix.patch    
Patch: Code
Approval: Triaged

 Description   

in src/jvm/clojure/lang/PersistentVector.java:168, there is an integer i being defined which is not being used anywhere in the method.

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/PersistentVector.java#L168



 Comments   
Comment by Stuart Halloway [ 31/Jan/14 6:14 PM ]

Smit, can you please submit a CA? http://clojure.org/contributing

Comment by Smit Shah [ 02/Feb/14 1:16 PM ]

Stuart, I will send the CA via post ASAP.
It might take a couple of days to reach Rich though.

Comment by Smit Shah [ 01/Mar/14 11:51 AM ]

Stuart, I have successfully submitted the CA (http://clojure.org/contributing).
I guess now merging this patch shouldn't be a problem

Comment by Alex Miller [ 02/Mar/14 11:37 AM ]

Thanks Smit!





[CLJ-1326] Inconsistent reflection warnings when target is a literal Created: 19/Jan/14  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   
user=> (set! *warn-on-reflection* true)
true
user=> (.get {} 0)
Reflection warning, NO_SOURCE_PATH:2:1 - call to get can't be resolved.
nil
user=> (.get {1 1} 0)
nil
user=> (.get ^:foo {1 1} 0)
Reflection warning, NO_SOURCE_PATH:4:1 - call to get can't be resolved.
nil
user=> (.get {1 (inc 0)} 0)
Reflection warning, NO_SOURCE_PATH:5:1 - call to get can't be resolved.
nil

Similar issues apply to other literals (vector literals, list literals)






[CLJ-1314] Correct placement of doc string for function bubble-max-key Created: 23/Dec/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring

Attachments: File clj-1314-v2.diff    
Patch: Code

 Description   

It is private, defined with defn-, so perhaps the doc string is superfluous, but someone wrote one, and it is in the wrong place relative to the args:

(defn- bubble-max-key [k coll]
  "Move a maximal element of coll according to fn k (which returns a number) 
   to the front of coll."
  (let [max (apply max-key k coll)]
    (cons max (remove #(identical? max %) coll))))

Found using a pre-release version of the Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 23/Dec/13 4:11 PM ]

Patch clj-1314-v1.diff simply corrects the location of the doc string for bubble-max-key.

Comment by Andy Fingerhut [ 02/Jan/14 11:05 AM ]

Patch clj-1314-v2.diff is same as the previous clj-1314-v1.diff, except it leaves out some unintended changes.





[CLJ-1292] print docstring should specify nil return value Created: 01/Nov/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring, print


 Description   

The docstring for print does not mention its return value. The docstring should clarify whether print dependably returns nil or shouldn't be depended on to (lest, for example, something leak out as the inadvertent return value of print's caller).






[CLJ-1291] struct-map docstring incomplete, inconsistent Created: 01/Nov/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Trivial
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

The docstring for struct-map refers to "structure-basis" and "keyvals" while the parameters are "s" and "inits". The docstring says "keyvals can also contain keys not in the basis" but does not say what happens in that case.






[CLJ-1240] Clarify limits of clojure.walk/macroexpand-all in docstring Created: 29/Jul/13  Updated: 03/Sep/13

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

Type: Enhancement Priority: Trivial
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: walk

Attachments: Text File 0001-CLJ-1240-Note-limits-of-clojure.walk-macroexpand-all.patch    
Patch: Code

 Description   

The clojure.walk/macroexpand-all function appears to be a general recursive macroexpansion, but it is not because it doesn't understand special forms such as let.

Current patch: 0001-CLJ-1240-Note-limits-of-clojure.walk-macroexpand-all.patch

The modified docstring in this patch notes that clojure.walk/macroexpand-all is not identical to macroexpansion by the Clojure compiler and should be used for development only.






[CLJ-1167] repl value of *file* is "NO_SOURCE_PATH", of *source-path* is "NO_SOURCE_FILE" Created: 19/Feb/13  Updated: 03/Sep/13

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

Type: Defect Priority: Trivial
Reporter: Brian Marick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl


 Comments   
Comment by Brian Marick [ 19/Feb/13 4:22 PM ]

Forgot to mention: I think it's intended to be the other way around, given the names.





[CLJ-1151] Minor Code Cleanup in core.reducers: use required walk, drop this for coll Created: 21/Jan/13  Updated: 03/Sep/13

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

Type: Enhancement Priority: Trivial
Reporter: Stefan Kamphausen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File tiny-reducers-cleanup.diff    
Patch: Code

 Description   

First, core.reducers requires clojure.walk :as walk, but does not use the alias.
Second, the two arity implementation of coll-reduce in function reducer uses 'this', whereas similar implementations in that file use 'coll'. AFAICT it makes no difference to use 'coll' (all tests pass, no change in performance) and it is more in line with the rest of the code.

The two things seem small enough to be put into one cleanup case.



 Comments   
Comment by Jozef Wagner [ 03/Sep/13 6:47 AM ]

your patch is wrong. If you want to replace this with coll, you have to allso call xf on f1.

(clojure.core.protocols/coll-reduce this f1 (f1))

becomes

(clojure.core.protocols/coll-reduce coll (xf f1) (f1))
Comment by Stefan Kamphausen [ 03/Sep/13 2:45 PM ]

Scary, that the test suite did not detect that.

Comment by Andy Fingerhut [ 03/Sep/13 2:59 PM ]

Is it straightforward to add a test that would have detected that?

Comment by Stefan Kamphausen [ 03/Sep/13 4:09 PM ]

I will happily look into that. It may take a few days, before I'll find the time, though.





[CLJ-1146] Symbol name starting with digits to defn throws "Unmatched delimiter )" Created: 13/Jan/13  Updated: 18/Apr/14

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

Type: Enhancement Priority: Trivial
Reporter: Linus Ericsson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, reader
Environment:

$java -jar clojure-1.5.0-RC2.jar

$java -version
java version "1.6.0_37"
Java(TM) SE Runtime Environment (build 1.6.0_37-b06-434-10M3909)
Java HotSpot(TM) 64-Bit Server VM (build 20.12-b01-434, mixed mode)
Mac OS X:
System Version: Mac OS X 10.6.8 (10K549)
Kernel Version: Darwin 10.8.0



 Description   

When trying to use an invalid symbol name when defining a function, the error message thrown is a confusing and wrong one. The error message is "RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:219)", which unfortunately is the only message seen in nrepled emacs.

$ java -jar clojure-1.5.0-RC2.jar
Clojure 1.5.0-RC2
user=> (defn 45fn [] nil)
NumberFormatException Invalid number: 45fn clojure.lang.LispReader.readNumber (LispReader.java:255)
[]
nil
RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:219)

Expected:
When trying to (defn or (def a thing with a non valid symbol name, the last thrown error message should be one stating that the given symbol name is not a valid one.



 Comments   
Comment by Kevin Downey [ 18/Apr/14 2:27 AM ]

this is an artifact of how streams and repls work.

when you type (defn 45fn [] nil) and hit enter, the inputstream flushes and "(defn 45fn [] nil)" is made available to the reader, the reader reads up to 45fn, throws an error back to the main repl loop, which prints out the error, then calls read, which still has the unread parts available to it "[] nil)"

changing this behavior would require significant changes to clojure's repl.

checkout https://github.com/trptcolin/reply instead





[CLJ-1115] multi arity into Created: 25/Nov/12  Updated: 06/Aug/14

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

Type: Enhancement Priority: Trivial
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: File multi-arity-into.diff    
Patch: Code and Test

 Description   

Any reason why into isn't multi arity?

(into to & froms) => (reduce into to froms)

(into #{} [3 3 4] [2 1] ["a"]) looks better than (reduce into #{} [[3 3 4] [2 1] ["a"]])



 Comments   
Comment by Timothy Baldridge [ 27/Nov/12 11:25 AM ]

Seems to be a valid enhancement. I can't see any issues we'd have with it. Vetted.

Comment by Timothy Baldridge [ 29/Nov/12 2:06 PM ]

Added patch & test. This patch retains the old performance characteristics of into in the case that there is only one collection argument. For example: (into [] [1 2 3]) .

Since the multi-arity version will be slightly slower, I opted to provide it as a second body instead of unifying both into a single body. If someone has a problem with this, I can rewrite the patch. At least this way, into won't get slower.

Comment by Rich Hickey [ 09/Dec/12 7:39 AM ]

This is a good example of an idea for an enhancement I haven't approved, and thus is not yet vetted.

Comment by Yongqian Li [ 05/Feb/14 1:16 AM ]

What's the plan for this bug?

Comment by Alex Miller [ 05/Feb/14 11:27 AM ]

@Yongqian This ticket is one of several hundred open enhancements right now. It is hard to predict when it will be considered. Voting (or encouraging others to vote) on an issue will raise it's priority. The list at http://dev.clojure.org/display/community/Screening+Tickets indicates how we prioritize triage.

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

Patch multi-arity-into.diff dated Nov 29 2012 no longer applied cleanly to latest Clojure master due to commits made earlier today. Those changes add another arity to the function clojure.core/into, so the multi-arity generalization of into seems unlikely to be compatible with those changes.





[CLJ-1095] Allow map-indexed to accept multiple collections (a la map) Created: 25/Oct/12  Updated: 08/Nov/12

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

Type: Enhancement Priority: Trivial
Reporter: Bo Jeanes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-map-indexed-accepts-multiple-collections.patch     Text File 0002-Add-test-for-multi-collection-map-indexed-fn.patch    
Patch: Code and Test

 Description   

Bring external interface of map-indexed in line with map. Existing usages of map-indexed unchanged both in implementation and interface.

examples
(map vector (range 10 20) (range 30 35)) ;=> ([10 30] [11 31] [12 32] [13 33] [14 34])
(map-indexed vector (range 10 20) (range 30 35)) ;=> ([0 10 30] [1 11 31] [2 12 32] [3 13 33] [4 14 34])

The attached patch is not necessarily the best implementation (I haven't benchmarked it or tried any alternatives yet) but hopefully enough to start a conversation about whether this is an addition that is warranted. I know I wished for this behavior a few weeks ago though I ended up finding another way.

(I haven't sent my CA yet, but I have it signed and ready to send in the next few days)



 Comments   
Comment by Aaron Bedra [ 25/Oct/12 5:11 PM ]

Can you add a test for the improved functionality?

Comment by Bo Jeanes [ 25/Oct/12 5:20 PM ]

You bet. I tried to before submitting this but found no existing tests for map-indexed to expand upon. Given that, I decided to just start the conversation first. If you think this is a good addition, I'll find a place to stick the tests and add a new patch file.

Comment by Bo Jeanes [ 25/Oct/12 8:05 PM ]

Add two unit tests for map-indexed. One tests old behavior (single collection) and the second tests mapping across 3 collections.

There were no existing tests for map-indexed that I could see to expand upon (using git grep map-indexed src/clojure)





[CLJ-1088] repl/source could support protocol functions Created: 21/Oct/12  Updated: 31/Jan/14

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

Type: Enhancement Priority: Trivial
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: repl

Attachments: Text File 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch    
Patch: Code

 Description   
user=> (source clojure.core.protocols/coll-reduce)
Source not found

But since the protocol fn's var's metadata points to the protocol var, and the protocol var knows the file and line where it was defined, it would be trivial to improve 'source' to look like this:

user=> (source clojure.core.protocols/coll-reduce)
(defprotocol CollReduce
  "Protocol for collection types that can implement reduce faster than
  first/next recursion. Called by clojure.core/reduce. Baseline
  implementation defined in terms of Iterable."
  (coll-reduce [coll f] [coll f val]))


 Comments   
Comment by Chouser [ 21/Oct/12 10:00 AM ]

Add one-line patch to clojure.repl/source so that it will find the protocol definition for a given protocol function.

Comment by Andy Fingerhut [ 31/Jan/14 6:25 PM ]

Patch 0001-Add-support-for-protocol-fns-to-repl-source.-CLJ-1088.patch no longer applies cleanly as of commits made to Clojure master on Jan 31 2014, probably due to the patch for CLJ-1176. I have not investigated how easy or difficult it would be to update.





[CLJ-1060] 'list*' returns not a list Created: 03/Sep/12  Updated: 18/Jan/13

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

Type: Defect Priority: Trivial
Reporter: Andrei Zhlobich Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File list-star-fix.diff    
Patch: Code and Test

 Description   

Function 'list?' returns sequence, but not a list.
It is a bit confusing.

user=> (list? (list* 1 '(2 3)))
false


 Comments   
Comment by Stuart Halloway [ 17/Sep/12 6:52 AM ]

should the docstring for list* change to say it returns a seq?

Comment by Timothy Baldridge [ 27/Nov/12 11:58 AM ]

Is there a reason why we can't have list* actually return a list? The cost of creating a list vs a cons is negligible.

Comment by Marek Srank [ 04/Jan/13 2:02 PM ]

The question is what to do with the one-argument case of list*, because in cases like: (list* {:a 1 :b 2}) it doesn't return IPersistentList as well. I propose just applying 'list'.

I added patch 'list-star-fix.diff' (dated 04/Jan/2013) with Cons implementing IPersistentList and doing (apply list args) in one-argument case of list*. To be able to use 'apply' in list* I had to declare it before the definition of list* in the source code. The apply itself also uses list*, but luckily not the one-argument version of list*, so it should be safe... The patch also contains simple unit tests.

Discussion is also here: https://groups.google.com/forum/#!topic/clojure/co8lcKymfi8

Comment by Michał Marczyk [ 04/Jan/13 4:11 PM ]

(apply list args) would make (list* (range)) hang, where currently it returns a partially-realized lazy seq. Also, even for non-lazy seqs – possibly lists themselves – it would always build a new list from scratch, right?

Also, if I'm reading the patch correctly, it would make 2+-ary list* overloads and cons return lists – that is, IPersistentList instances – always (Conses would now be lists), but repeatedly calling next on such a list might eventually produce a non-list. The only way around that would be to make all seqs into IPersistentLists – that doesn't seem desirable at first glance...?

On a side note, for the case where the final "args" argument is in fact a list, we already have a "prepend many items" function, namely conj. list* currently acts as the vararg variant of cons (in line with Lisp tradition); I'm actually quite happy with that.

Comment by Michał Marczyk [ 04/Jan/13 4:19 PM ]

I'm in favour of the "list" -> "seq" tweak to the docstring though, assuming impl remains unchanged.

Comment by Marek Srank [ 04/Jan/13 6:13 PM ]

Yep, these are all valid points, thanks! I see this as a question whether the list* function is a list constructor or not. If yes (and it would be possible to implement it in a satisfactory way), it should probably return a list.

We could avoid building a new list by sth like:

(if (list? args)
  args
  (apply list args))

(btw, 'vec' also creates a new vector even when the argument itself is a vector)

The contract of next seems to be to return a seq, not a list - its docstring reads: "Returns a seq of the items after the first. Calls seq on its argument. If there are no more items, returns nil."

Btw, in some Lisp/Scheme impls I checked, cons seems to be a list as well. E.g. in CLisp (and similar in Guile and Racket):

> (listp (cons 2 '()))
T
> (listp (list* 1 2 '()))
T




[CLJ-1057] Var's .setDynamic does not set :dynamic in metadata Created: 02/Sep/12  Updated: 03/Dec/12

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

Type: Defect Priority: Trivial
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   
((juxt (comp :dynamic meta) #(.isDynamic %)) #'*agent*)


 Comments   
Comment by Timothy Baldridge [ 03/Dec/12 9:10 AM ]

This is actually an enhancement as no where in the clojure code is provision made for syncing var's metadata and dynamic state. .isDynamic is the authoritative source, and the calling of .setDynamic is configured by the compiler. If you'd like to see this change, please, feel free to bring it up on clojure-dev for a discussion.





[CLJ-1044] Enable refering to ->type inside deftype Created: 18/Aug/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Trivial
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: deftype

Attachments: File 001-enable-factory-ctor-inside-deftype.diff    
Patch: Code

 Description   

Inside defrecord is possible to refer to ->type-ctor, that is not possible inside deftype.
This patch adds an implicit declare, as done in defrecord



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

Seems valid. Vetting.





[CLJ-1027] Outdated documentation for gen-class's :exposes-methods option Created: 18/Jul/12  Updated: 03/Sep/13

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

Type: Defect Priority: Trivial
Reporter: Dan Lidral-Porter Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

The docstring for gen-class says the following regarding the :exposes-methods option:

"It is sometimes necessary to call the superclass' implementation of an
overridden method. Those methods may be exposed and referred in
the new method implementation by a local name."

To me, this suggests that supplying something like `{foo fooSuper}` allows me to use the symbol `fooSuper` in my new method implementation. Doing this actually results in an error while compiling because `fooSuper` cannot be resolved. It seems that what actually happens is that a `fooSuper` instance method is defined, which calls the superclass's implementation. The docstring should be updated to reflect this.






[CLJ-1020] clojure.inspector/inspect-table gives up when first element of coll is nil Created: 02/Jul/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Trivial
Reporter: Dimitrios Piliouras Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Ubuntu 12.04, Java 7, Clojure 1.4


Attachments: Text File clj-1020-inspect-table-skip-nil-rows-patch1.txt     Text File clj-1020-inspect-table-skip-nil-rows-patch2.txt     Text File inspector.patch    
Patch: Code

 Description   

clojure.inspector/inspect-table gives up when first element of coll is nil. The patch provided is rather trivial...instead of blindly choosing the first element (which might be nil), it would be more convenient to choose the first element that is NOT nil and use its keys for columns...a similar issue exists with clojure.pprint/print-table where the keys of the first element are used (if not provided explicitly). The same is not true for 'inspect-table' though. As a result, one cannot 'inspect' a collection of maps where the first element is nil. My (trivial) patch looks for the first element which is NOT nil and uses its keys instead. Maps have to have the same length anyway so no problems there...



 Comments   
Comment by Andy Fingerhut [ 12/Jul/12 1:01 PM ]

clj-1020-inspect-table-skip-nil-rows-patch1.txt of July 12, 2012 is identical to inspector.patch of July 2, 2012, except it is in the desired git format. Proper attribution is given to author Dimitrios Piliouras in the patch.

Comment by Dimitrios Piliouras [ 02/Aug/13 9:17 AM ]

I literally copied the contents of "clj-1020-inspect-table-skip-nil-rows-patch1.txt" and pasted them on "clj-1020-inspect-table-skip-nil-rows-patch2.txt" only changing:

(some #(when-not (nil? %) %) data)
to
(some identity data)

much nicer/cleaner and faster since we skip the internal nil-test.





[CLJ-1017] Metadata expressions are evaluated after the expression they affect Created: 23/Jun/12  Updated: 23/Jun/12

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

Type: Defect Priority: Trivial
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Repro:

user=> (def x (atom 1))
#'user/x
user=> ^{:foo (swap! x inc)} {:bar (swap! x inc)}
{:bar 2}
user=> (meta *1)
{:foo 3}

Presumably this is because ^:foo x expands to (with-meta x {:foo true}) but probably should have reversed argument order or use a let expression.






[CLJ-738] <= is incorrect when args include Double/NaN Created: 14/Feb/11  Updated: 25/Apr/14

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

Type: Defect Priority: Trivial
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: math
Environment:

Mac OS X, Java 6


Attachments: File 738.diff     File 738-tests.diff     File clj-738-v2.diff    
Patch: Code and Test
Approval: Screened

 Description   
user=> (<= Double/NaN 1)
false  
user=> (<= (double Double/NaN) 1)
true    ;; should match Double object result

Cause: The problem was that the logic for lte/gte depended on the fact that lte is equivalent to !gt.
However, in Java, this assumption is invalid - any comparison involving NaN always yields false.

Solution: The fix was to adding lte and gte methods to Numbers.Ops directly, rather than implementing everything in terms of lt. This was the only fix I could see that didn't incur the cost of runtime checks for NaN.

Patch: clj-738-v2.diff

Screened by: Alex Miller



 Comments   
Comment by Jason Wolfe [ 14/Feb/11 7:18 PM ]

The source of the issue seems to be incorrect treatment of boxed NaN:

user> (<= 1000 (Double. Double/NaN))
true
user> (<= 1000 (double Double/NaN))
false

Comment by Alexander Taggart [ 28/Feb/11 11:14 PM ]

Primitive comparisons use java's primitive operators directly, which always return false for NaN, even when testing equality between two NaNs.

In clojure, Number comparisons are all logical variations around calls to Numbers.Ops.lt(Number, Number). So a call to (<= x y) is actually a call to (not (< y x)), which eventually uses the primitive < operator. Alas that logical premise doesn't hold when dealing with NaN:

user=> (<= 1 Double/NaN)
false
user=> (not (< Double/NaN 1))
true

So the bug is not that boxed NaN is treated incorrectly, but rather:

user> (<= 1000 (Double. Double/NaN)) ; becomes !(NaN < 1000) 
true
user> (<= 1000 (double Double/NaN))  ; becomes (1000 <= NaN)
false

In the original example, since there are more than two args, the primitive looking args were boxed:

user=> (<= 10 Double/NaN 1) ; equivalent to logical-and of the following
true
user=> (<= 10 (Double. Double/NaN))  ; becomes !(NaN < 10)
true
user=> (<= (Double. Double/NaN) 1)   ; becomes !(1 < NaN)
true

Note however that java object comparisons for NaNs behave differently: NaN is the largest Double, and NaNs equal each other (see the javadoc).

If we make object NaN comparisons always return false, we would need to add the rest of the comparison methods to Numbers.Ops. Yet doing so could also make collection sorting algorithms behave oddly, deviating from sorting written in java. Besides, (= NaN NaN) => false is annoying.

Clojure already throws out the notion of error-free dividing by zero (which for doubles would otherwise result in NaN or Infinity, depending on the dividend). Perhaps we could similarly error on NaNs passed to clojure numeric ops. They seem to be more trouble than they're worth. That said, people smarter than me thought they were useful.

Then there's that -0.0 nonsense...

Comment by Jouni K. Seppänen [ 19/Mar/11 3:02 PM ]

On current master, (<= x y) seems to be special-cased by the compiler, but when <= is called dynamically, the bug is still there:

user=> (<= 1 Float/NaN)
false
user=> (let [op <=] (op 1 Float/NaN))
true

Since CLJ-354 got marked "Completed", perhaps there was an attempt to fix this.

Comment by Alexander Taggart [ 19/Mar/11 6:45 PM ]

Using let forces calling <= as a function rather than inlining Numbers/lte, which means the args are treated as objects not primitives, thus the different behaviour as I discussed earlier.

Comment by Aaron Bedra [ 28/Jun/11 6:28 PM ]

Rich, what should the behavior be?

Comment by Jouni K. Seppänen [ 29/Jun/11 1:22 AM ]

My suggestion for the behavior is to follow Java (Java Language Specification §15.20.1) and IEEE 754. The java.sun.com site seems to be down right now, but here's a Google cache link:

http://webcache.googleusercontent.com/search?q=cache:http://java.sun.com/docs/books/jls/third_edition/html/expressions.html#15.20.1

Comment by Rich Hickey [ 29/Jul/11 7:48 AM ]

It should work the same as primitive double in all cases

Comment by Luke VanderHart [ 26/Aug/11 11:33 AM ]

Added patches. The problem was that our logic for lte/gte depended on the fact that lte is equivalent to !gt.

However, in Java, this assumption is invalid - any comparison involving NaN always yields false.

The fix was to adding lte and gte methods to Numbers.Ops directly, rather than implementing everything in terms of lt. This was the only fix I could see that didn't incur the cost of runtime checks for NaN.

Comment by Alex Miller [ 04/Mar/14 3:18 PM ]

David Welte noted: "CLJ-738 is marked Closed but the attached patch is has not been applied and both Clojure 1.5.1 and 1.6.0-beta2 exhibit the bad behavior listed in CLJ-738. The issue that CLJ-738 is that (<= (Double. Double/NaN) 1) evaluates to true while (<= Double/NaN 1) evaluates to false."

I concur that this patch was not applied. It looks likely that Luke marked this as Resolved when the patch was ready instead of whatever state change would have been appropriate at the time of the ticket (the process has varied over the years). AFAICT, this ticket should be open and Vetted (accepted as a problem) but probably needs release targeting and an updated patch based on current code.

Comment by Andy Fingerhut [ 05/Mar/14 12:32 PM ]

Might want to make the "Fix Version" on this ticket empty so it is back on the JIRA state chart as Vetted.

Comment by Andy Fingerhut [ 18/Apr/14 11:41 AM ]

Patch clj-738-v2.diff is identical in content to Luke's 2 patches 738.diff and 738-tests.diff, and includes them both, retaining his authorship. The only change is to a few context lines so that the new patch applies cleanly to latest master, whereas the older patches did not.

Comment by Alex Miller [ 24/Apr/14 3:22 PM ]

While the patch looks good and tests all pass, the example listed in the ticket description did not actually change behavior with the patch?

Comment by Jason Wolfe [ 24/Apr/14 5:19 PM ]

The ticket description has a typo (long, not double) – sorry. The first comment has a real test case.

user> (<= 1000 (Double. Double/NaN))
true
user> (<= 1000 (double Double/NaN))
false

Comment by Alex Miller [ 24/Apr/14 8:22 PM ]

Doh! Thank you. I'm the one that introduced it too.





[CLJ-704] range function has missing documentation Created: 04/Jan/11  Updated: 14/Jun/14

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

Type: Enhancement Priority: Trivial
Reporter: Maarten Hus Assignee: Plínio Balduino
Resolution: Unresolved Votes: 0
Labels: docstring
Environment:

All


Approval: Triaged

 Description   

The range function's documentation does indicate the following usage:

(range 10 0 -1) -> (10, 9, 8, 7, 6, 5, 4, 3, 2, 1)

Current doc:

Returns a lazy seq of nums from start (inclusive) to end
(exclusive), by step, where start defaults to 0, step to 1, and end
to infinity.

Suggestion:

Returns a lazy seq of nums from start (inclusive) to end
(exclusive), by step, where start defaults to 0, step to 1, and end
to infinity.

Its also possible to step down rather than up, for example counting
backwards from 10 by -1: (range 10 0 -1).



 Comments   
Comment by Rasmus Svensson [ 15/Jan/11 7:39 AM ]

The current doc actually mentions the 'step' parameter briefly:

"[...] to end (exclusive), by step, where start [...]"

But as this might be easy to miss, an addition to the doc is still a good idea, I think.

My suggestion:

Returns a lazy seq of nums from start (inclusive) to end
(exclusive), by step, where start defaults to 0, step to 1, and end
to infinity. Step may be negative to count backwards.

Comment by Plínio Balduino [ 13/Jun/14 10:59 PM ]

There was any news about it?

Could I assign it to me?

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

No, no news about this one. It is in the 'open' state, meaning that there is currently no judgement as to whether Clojure screeners or Rich Hickey are interested in such a change. http://dev.clojure.org/display/community/JIRA+workflow

That said, it seems like it should not take a lot of time to create a patch. Detailed instructions are given here: http://dev.clojure.org/display/community/Developing+Patches

Comment by Alex Miller [ 14/Jun/14 7:34 AM ]

Go for it!

Comment by Plínio Balduino [ 14/Jun/14 10:57 AM ]

I cannot reproduce this: "When step is equal to 0, returns an infinite sequence of start."

https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L2726-L2729

Is this correct?

Comment by Andy Fingerhut [ 14/Jun/14 2:47 PM ]

The last time range was modified was due to ticket CLJ-1018. See the attached patch there, which I believe is the one that was applied after Clojure 1.5 but before Clojure 1.6.

Perhaps that change does not do what it claimed to do in the doc string. I haven't looked at it in detail yet.

Comment by Andy Fingerhut [ 14/Jun/14 2:54 PM ]

In Clojure 1.6, it appears that it may be more accurate if the last two sentences in the doc string were modified. range's doc string is currently:

Returns a lazy seq of nums from start (inclusive) to end
(exclusive), by step, where start defaults to 0, step to 1, and end to
infinity. When step is equal to 0, returns an infinite sequence of
start. When start is equal to end, returns empty list.

It might be more accurate to say:

Returns a lazy seq of nums from start (inclusive) to end
(exclusive), by step, where start defaults to 0, step to 1, and end to
infinity. When start is equal to end, returns empty list. When step
is equal to 0 and start and end differ, returns an infinite sequence of
start.





[CLJ-107] GC Issue 103: bit-count function Created: 17/Jun/09  Updated: 22/Oct/13

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

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

Attachments: File clj-107-v1.diff    
Patch: Code and Test

 Description   
Reported by a...@thened.net, Apr 08, 2009

I posted this small patch to the mailing list last week but received no
feedback, so I'm attaching it here to make sure it doesn't get lost.  I
have submitted a CA.

http://groups.google.com/group/clojure/browse_thread/thread/4345f76a12bac6fe/

The new function bit-count returns the count of 1-bits in a number, like
C's popcount or Common Lisp's logcount.


 Comments   
Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/107
Attachments:
bit-count.diff - https://www.assembla.com/spaces/clojure/documents/dqone2w4er3RbzeJe5afGb/download/dqone2w4er3RbzeJe5afGb

Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

oranenj said: [file:dqone2w4er3RbzeJe5afGb]

Comment by Assembla Importer [ 24/Aug/10 3:45 AM ]

richhickey said: Updating tickets (#8, #19, #30, #31, #126, #17, #42, #47, #50, #61, #64, #69, #71, #77, #79, #84, #87, #89, #96, #99, #103, #107, #112, #113, #114, #115, #118, #119, #121, #122, #124)

Comment by Andy Fingerhut [ 15/Nov/12 8:40 PM ]

clj-107-add-bit-count-v1.txt is probably a correct updated version of the old patch linked above. Added a couple of unit tests.

Comment by Andy Fingerhut [ 22/Oct/13 7:56 PM ]

Patch clj-107-v1.diff is identical to earlier clj-107-add-bit-count-v1.txt, but with .diff suffix for easier diff-style viewing in some editors.





Generated at Fri Aug 22 18:42:06 CDT 2014 using JIRA 4.4#649-r158309.