<< Back to previous view

[UNIFY-6] Create tests for k&v map unification Created: 25/May/12  Updated: 25/May/12

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Substitution in map keys or values should both work. I doubt there are tests around this.






[UNIFY-5] core.unify locks Midje down to Clojure 1.3 Created: 26/Apr/12  Updated: 25/May/12  Resolved: 25/May/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Brian Marick Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

According to `lein2 deps :tree` core.unify 0.5.2 is the (a?) reason that Midje can't use Clojure 1.4.



 Comments   
Comment by Fogus [ 26/Apr/12 10:23 AM ]

I will put together a point release for the next version today or tomorrow (latest). Thank you.

Comment by Fogus [ 25/May/12 10:01 PM ]

Tomorrow came and went. I apologize. The 0.5.3 version has been deployed and should be available on Maven Central in the next few hours.





[UNIFY-4] Vectors of different length incorrectly unify Created: 08/Feb/12  Updated: 25/May/12  Resolved: 25/May/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Thomas Winant Assignee: Fogus
Resolution: Completed Votes: 0
Labels: unify


 Description   
(unify '[1 ?x] '[1])
=> {?x nil}

I expect this unification to fail.



 Comments   
Comment by Fogus [ 26/Apr/12 11:48 AM ]

Which version are you using?

Comment by Thomas Winant [ 26/Apr/12 1:43 PM ]

Version 5.2

Comment by Fogus [ 25/May/12 10:00 PM ]

This is not a perfect solution, but it works as a stopgap as this will be handled in the next 0.1.0 version. For now the 0.5.3 version has been deployed and should be available on Maven Central in the next few hours.





[UNIFY-3] Enhance documentation Created: 03/Feb/12  Updated: 03/Feb/12

Status: Open
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 0
Labels: docs


 Description   

The current unify docs are spartan and "just the facts". It would be useful to have a set of docs that:

  • explain unification
  • explain the library use cases
  • show a simple example use





[UNIFY-2] Remove reflection warnings Created: 05/Jan/12  Updated: 05/Jan/12  Resolved: 05/Jan/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: performance, reflection, unify

Approval: Ok

 Description   
Reflection warning, clojure/core/unify.clj:30 - reference to field getClass can't be resolved.
Reflection warning, clojure/core/unify.clj:30 - reference to field isArray can't be resolved.

These reflective calls occur frequently enough that they should be resolved.



 Comments   
Comment by Fogus [ 05/Jan/12 7:37 AM ]

Fixed in 6b6d1130bf857439d1863f6fc574a7a6541b84b8.





[UNIFY-1] Cut 0.5.2 release jar Created: 14/Dec/11  Updated: 09/Jan/12  Resolved: 09/Jan/12

Status: Resolved
Project: core.unify
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Fogus Assignee: Fogus
Resolution: Completed Votes: 0
Labels: None


 Description   

There are a couple of bug fixes that should make it into a +0.0.1 release.






[TTRACE-2] Eliminate a few uses of Reflection in tools.trace Created: 28/Oct/12  Updated: 01/Dec/12  Resolved: 01/Dec/12

Status: Resolved
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File ttrace-2-eliminate-reflection-v1.txt    
Patch: Code

 Description   

There are a few instances of reflection in tools.trace code that can be eliminated with suitable type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 10:32 PM ]

ttrace-2-eliminate-reflection-v1.txt dated Oct 28 2012 eliminates all current uses of reflection in tools.trace code by adding type hints.

Comment by Luc Préfontaine [ 14/Nov/12 6:14 PM ]

Done in 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:36 PM ]

Issued 0.7.5, mising readme but was fixed in 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:37 PM ]

Fixed in 0.7.5





[TTRACE-1] Two convenience predicate functions useful for IDE-tools Created: 23/Apr/12  Updated: 01/Dec/12  Resolved: 01/Dec/12

Status: Resolved
Project: tools.trace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Frank Siebenlist Assignee: Luc Préfontaine
Resolution: Completed Votes: 0
Labels: enhancement

Patch: Code and Test

 Description   

In our IDE-tool clj-ns-browser, we show the user a list of vars and make it easy to add a trace by selecting a var and clicking a button.

In order to only enable the button for a var that is traceable, we use predicate "var-traceable?".

In order to change the button text from "trace" to "untrace", we use predicate "var-traced?".

Both "var-traceable?" and "var-traced?" are aware of tools.trace internals - ideally, we would like to remain oblivious to trace's inners...

Our implementations for the predicates is at: https://gist.github.com/2472261

Thanks for the great tool!



 Comments   
Comment by Luc Préfontaine [ 14/Nov/12 5:56 PM ]

Done, however the names where changed to traced? and traceable?.

Comment by Luc Préfontaine [ 14/Nov/12 6:18 PM ]

Done is 0.7.4

Comment by Luc Préfontaine [ 01/Dec/12 3:38 PM ]

Done, "good" version is 0.7.5 (readme & comments need an update in 0.7.4")





[TRDR-6] Some uses of reflection in tools.reader code slow it down unnecessarily Created: 13/Apr/13  Updated: 13/Apr/13  Resolved: 13/Apr/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt    
Patch: Code

 Description   

Attached patch uses type hints to eliminate several instances of reflection in the tools.reader code.

FYI, you can run 'lein check' to cause Leiningen to compile the code with warn-on-reflection true, I believe for every source file (not sure about the tests, but reflection isn't such a big deal in them anyway).



 Comments   
Comment by Nicola Mometto [ 13/Apr/13 12:13 PM ]

Andy, I don't see any patch attached to this ticket, I think you forgot to attach it.

(P.S. thanks, I didn't know about 'lein check')

Comment by Andy Fingerhut [ 13/Apr/13 12:19 PM ]

Patch trdr-6-eliminate-reflection-with-type-hints-patch-v1.txt dated Apr 13 2013 eliminates all occurrences of reflection found in latest version of tools.reader. Please check them carefully before committing them, especially the ones in default_data_readers.clj.

And I know the reflection warnings in default_data_readers.clj exist in Clojure's code, too, where you copied those from. CLJ-1080 has a patch addressing those and many other reflections within Clojure's code.

Comment by Nicola Mometto [ 13/Apr/13 12:59 PM ]

Andy, casting to char makes tools.reader crash under clojure-1.3, apparently casting to char is possible only from clojure 1.4.

Could you please submit a patch removing reflection in default_data_readers.clj and the docstring fixes only while I try to figure out a way to maintain clojure 1.3 compatibility and remove the reflection? (or if you have an idea on how to do it, you're more than welcome )

EDIT:
I edited your patch manually, and pushed a commit to type hint to char only for >clojure-1.3.0
I don't know if there is a way to avoid the reflection from clojure 1.3.0, but for the all the other versions of clojure all reflection is gone, thanks!

Comment by Andy Fingerhut [ 13/Apr/13 2:43 PM ]

Great. I was off-line there for a while, but glad you noticed the 1.3 compatibility issue where I didn't, and glad you found a different way to eliminate reflection with 1.4 and later.





[TRDR-5] Additional "Differences from LispReader.java" for README Created: 08/Apr/13  Updated: 10/Apr/13  Resolved: 10/Apr/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Suggested addition to "Differences from LispReader.java" section of README.md:

read is capable of reading the symbol / with an explicit namespace, e.g. foo//, whereas clojure.lang.LispReader/read throws an exception. Refer to CLJ-873. Except for this special case, read throws an exception if a symbol contains more than one / character, whereas clojure.lang.LispReader/read allows them, returning a symbol with one or more / characters in its namespace name.



 Comments   
Comment by Nicola Mometto [ 10/Apr/13 5:20 AM ]

Thanks, fixed https://github.com/clojure/tools.reader/commit/f689cb283d1fb539a6cabbefd4036f620dabe693





[TRDR-4] Typo: Change 'end' to 'edn' in 'clojure.tools.reader.end/read-string' in README Created: 15/Mar/13  Updated: 15/Mar/13  Resolved: 15/Mar/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

Just a miniscule typo



 Comments   
Comment by Nicola Mometto [ 15/Mar/13 9:50 AM ]

Fixed, thanks





[TRDR-3] Make read-line available in clojure.tools.reader and/or clojure.tools.reader.edn namespace? Created: 15/Mar/13  Updated: 15/Mar/13  Resolved: 15/Mar/13

Status: Closed
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None


 Description   

You mention the enhanced version of read-line that takes a reader argument in the README, but it seems like it is only available in the clojure.tools.reader.reader-types namespace. Is that intentional? Perhaps making it available in the clojure.tools.reader and/or clojure.tools.reader.edn namespace might be more convenient for the user of the library? I understand that the current implementation of read-line is Java-specific, so maybe this is the only reasonable way to expose it.

In any case, documenting the namespace in which this enhanced read-line is available in the README would be good.



 Comments   
Comment by Nicola Mometto [ 15/Mar/13 10:26 AM ]

I added more documentation on the readme, making it clear in which namespace read-line is defined.
(https://github.com/clojure/tools.reader#public-api, https://github.com/clojure/tools.reader#differences-from-lispreaderjava)

read-line definitely belongs in the c.t.r.reader-types namespace, since it works on reader-types and returns a string, so it's not a reader function.

I hope this addresses this ticket, I'm closing it, feel free to reopen it you think the doc needs to be more clear.

Comment by Andy Fingerhut [ 15/Mar/13 7:04 PM ]

Looks thoroughly documented to me – above and beyond what I would have asked for. Thanks.





[TRDR-2] Fix a few README typos Created: 14/Feb/13  Updated: 15/Feb/13  Resolved: 15/Feb/13

Status: Resolved
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Nicola Mometto
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-2-fix-readme-typos-patch-v1.txt    
Patch: Code

 Description   

There are a few typos in the README



 Comments   
Comment by Andy Fingerhut [ 14/Feb/13 3:16 PM ]

Nothing major here. Just a few typo fixes for the README that I found.

Comment by Nicola Mometto [ 15/Feb/13 4:19 AM ]

Thanks, applied





[TRDR-1] read-char returns nil for some input types, on first attempt to read a char Created: 12/Feb/13  Updated: 12/Feb/13  Resolved: 12/Feb/13

Status: Resolved
Project: tools.reader
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Andy Fingerhut
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File trdr-1-correct-read-char-v1.txt    
Patch: Code

 Description   

The return value from Java's .read method is -1 for EOF. A couple of condition checks in the code appear to be reversed. See the patch.



 Comments   
Comment by Nicola Mometto [ 12/Feb/13 4:47 PM ]

Thanks, tests for reader-types are now on the TODO list





[TNS-7] Stack overflow on refresh after circular dependency detected Created: 17/Apr/13  Updated: 17/Apr/13

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None
Environment:

tools.namespace 0.2.3



 Description   
user> (refresh)
Exception Circular dependency between com.example.foo and com.example.bar  clojure.tools.namespace.dependency.MapDependencyGraph (dependency.clj:74)
user> (refresh)
StackOverflowError   clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
user> (clojure.repl/pst)
StackOverflowError 
	clojure.lang.PersistentHashMap$BitmapIndexedNode.index (PersistentHashMap.java:576)
	clojure.lang.PersistentHashMap$BitmapIndexedNode.find (PersistentHashMap.java:676)
	clojure.lang.PersistentHashMap$ArrayNode.find (PersistentHashMap.java:396)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:152)
	clojure.lang.PersistentHashMap.valAt (PersistentHashMap.java:156)
	clojure.lang.RT.get (RT.java:645)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
	clojure.tools.namespace.dependency/transitive/fn--7043 (dependency.clj:51)
	clojure.core.protocols/fn--6022 (protocols.clj:79)
	clojure.core.protocols/fn--5979/G--5974--5992 (protocols.clj:13)
	clojure.core/reduce (core.clj:6177)
	clojure.tools.namespace.dependency/transitive (dependency.clj:52)
nil





[TNS-6] Attempt to reload deleted file Created: 14/Dec/12  Updated: 14/Dec/12

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I can't identify the exact circumstances, but I have seen events where a source code file has been deleted but clojure.tools.namespace.repl/refresh still tries to reload it. Because the file doesn't exist, there's an exception when you try to load it, so you're stuck.






[TNS-5] Allow any valid .clj* source file to be parsed/analysed Created: 01/Nov/12  Updated: 13/Dec/12

Status: Open
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Max Penet Assignee: Stuart Sierra
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: Text File e3cd6d1fa6e0c900bc1086e4a93bbc9cb343a820.patch    

 Description   

This broadens the allowed file types to anything ending with #"\.clj.?$", meaning this would work for clj, cljs, cljc, cljx and possibly other Clojure implementations with their own extension in the future.

This allows libraries such as codox (and possibly autodoc) to work with ClojureScript and others implementations without any modification.

Note: My CA is on the way, I sent it a week ago, I am not sure if it arrived yet (it was sent from Switzerland with normal mail, with an ETA of 1 week).



 Comments   
Comment by Max Penet [ 02/Nov/12 6:51 AM ]

CA received it seems (I am listed on http://clojure.org/contributing ).

Comment by Stuart Sierra [ 02/Nov/12 3:23 PM ]

I'm not sure about this. If you're only using c.t.n.find in isolation, it's fine. But if you're using code-reloading and c.t.n.repl, it could incorrectly try to reload .cljs files in JVM Clojure.

We really need [Feature Expressions]http://dev.clojure.org/display/design/Feature+Expressions or something like it to get away from multiple file extensions.

Until then, I think it has to be optional. I don't know how best to achieve this. The APIs in c.t.n.repl and c.t.n.dir are not amenable to extension. I'll think about it.

Comment by Max Penet [ 06/Nov/12 6:11 PM ]

True, I didn't realize that.

Maybe using a dynamic var to hold the regex pattern (or a predicate?) could be a reasonable solution in the meantime, this would allow to rebind it in the case of codox & similar libs.
I don't really "like" to use dynamic vars (or regexes!), but in this case it might make sense.
Not to mention it would also allow more flexibility on projects where you don't want to have your clj codoxed, but only your cljs for instance.

Comment by Max Penet [ 24/Nov/12 7:47 AM ]

Any thoughts on my last comment/edit? I don't think there is a single doc lib that works with cljs at the moment, it is a bit painful to be honest.

Comment by Stuart Sierra [ 24/Nov/12 4:25 PM ]

Yes, I think a dynamic var would be OK. However, I would like to know of a real use case, not just a potential one.

Comment by Max Penet [ 13/Dec/12 2:40 AM ]

The idea was to be able to use codox on cljs files, I tried locally but there are other problems with this approach to be able to get to cljs vars metadata. So in the end I think you were right, maybe it's better to wait for feature expressions for this.





[TNS-4] Eliminate several uses of reflection in tools.namespace Created: 28/Oct/12  Updated: 28/Oct/12  Resolved: 28/Oct/12

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File tns-4-eliminate-reflection-v1.txt    

 Description   

There are several reflection warnings when compiling tools.namespace, all of which can be eliminated with fairly straightforward additions of type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 8:08 PM ]

tns-4-eliminate-reflection-v1.txt dated Oct 28 2012 adds the necessary type hints to eliminate reflection warnings in tools.namespace.

Comment by Stuart Sierra [ 28/Oct/12 9:14 PM ]

Patch applied. Thanks!





[TNS-3] refresh-dirs not used Created: 08/Oct/12  Updated: 08/Oct/12  Resolved: 08/Oct/12

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None


 Description   

Submitted by Mika Raento as a GitHub pull request: https://github.com/clojure/tools.namespace/pull/1



 Comments   
Comment by Stuart Sierra [ 08/Oct/12 4:59 PM ]

Fixed in commit bc9d5c1a6f191070a425b04feda9e2d3c2eb6928

https://github.com/clojure/tools.namespace/commit/bc9d5c1a6f191070a425b04feda9e2d3c2eb6928





[TNS-2] Add function to parse dependencies from namespace declarations Created: 16/May/12  Updated: 16/May/12  Resolved: 16/May/12

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TNS-2-0001.patch    
Patch: Code




[TNS-1] Workaround to Clojure 1.2 reader bug Created: 14/Dec/11  Updated: 24/Apr/12  Resolved: 24/Apr/12

Status: Resolved
Project: tools.namespace
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sam Ritchie Assignee: Stuart Sierra
Resolution: Completed Votes: 2
Labels: None
Environment:

Mac OS X, Clojure 1.2.1, Leiningen 1.6.2


Attachments: Text File 0001-Workaround-to-Clojure-1.2-reader-bug.patch    
Patch: Fixed

 Description   

The clojure 1.2 reader will allow invalid forms like {:key} to be read in, and only throw an exception on printing. Currently clojure.tools.namespace calls (read rdr) within a try form; the bug means that this particular type of error is never caught. This patch forces the reader to try and resolve with str, allowing clojure.tools.namespace to catch and bury the error.

I was running into this with moustache templates from lein-newnew on the classpath; these contain namespace headers that look like (ns name.core). This would cause (clojure.tools.namespace/find-namespaces-on-classpath) to fail when printing its results but not when actually running.



 Comments   
Comment by Sam Ritchie [ 14/Dec/11 4:55 PM ]

Funny, jira picked up the moustache markup. a bad namespace looks like (ns { { name } } . core).

Comment by Sam Ritchie [ 11/Jan/12 3:59 PM ]

Ping – Stuart, any thoughts on this?

Comment by Stuart Sierra [ 11/Jan/12 6:09 PM ]

Why should tools.ns do this? If the syntax is wrong, it's wrong.

Comment by Sam Ritchie [ 11/Jan/12 6:43 PM ]

Because without this patch, it's impossible to catch and bury errors from invalid reader syntax. I believe this comes from a bug in the reader that was fixed with 1.2.

Comment by Stuart Sierra [ 27/Jan/12 9:44 AM ]

Declined. It is not the responsibility of this library to catch errors in old versions of Clojure.

Comment by Stuart Sierra [ 24/Apr/12 2:01 PM ]

Reopening because this is still a visible issue for some libraries. I still don't like it, but I'm going to include it.

Comment by Stuart Sierra [ 24/Apr/12 2:04 PM ]

Patch applied.

Comment by Sam Ritchie [ 24/Apr/12 2:07 PM ]

Great, thanks!





[TMACRO-2] protect let-bound symbols from macrolet expansion Created: 26/Nov/12  Updated: 27/Nov/12  Resolved: 27/Nov/12

Status: Resolved
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Tom Jack Assignee: Konrad Hinsen
Resolution: Completed Votes: 1
Labels: bug, patch

Attachments: Text File 0001-Don-t-apply-macro-expansion-to-protected-forms.patch     File macrolet-protect.diff    
Patch: Code and Test

 Description   

As discussed here: https://groups.google.com/d/msg/clojure-dev/-/UheAzkyI_WcJ

Patch macrolet-protect.diff fixes the issue and provides a test.



 Comments   
Comment by Tom Jack [ 26/Nov/12 2:55 PM ]

D'oh, patch macrolet-protect.diff has a bug — it doesn't macroexpand-1 if the symbol is protected.

Comment by Tassilo Horn [ 27/Nov/12 1:38 AM ]

This patch supersedes Tom's patch as discussed on the clojure-dev mailinglist. See

Message-ID: <CADygAw4ArNq4Z2=ZJmT6MwkBw160ShJmfQwoFEh4VOiwxfjDKQ@mail.gmail.com>

for reference.

The patch also adds letfn* as a form introducing protected symbols. That one was completely missing.

I added test cases for both let as well as letfn.

Comment by Konrad Hinsen [ 27/Nov/12 8:40 AM ]

Patch applied: https://github.com/clojure/tools.macro/commit/19c8197e10079f04e55d81c26884b9248762c2ca

Thanks!





[TMACRO-1] Preserve metadata for (at least) unexpanded forms Created: 02/Apr/12  Updated: 02/Apr/12

Status: Open
Project: tools.macro
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stephen Compall Assignee: Konrad Hinsen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.3.0, tools.macro 0.1.1, algo.monads 0.1.0



 Description   

In algo.monads:

(with-monad blah some-code...)

causes all of some-code to report as being on the line of the with-monad form. Could, perhaps, expand-all copy over metadata from the original form, at least in the common case that the form-at-point was not itself expanded?






[TLOG-12] Reflection Warnings when using tools.logging Created: 30/Apr/13  Updated: 30/Apr/13

Status: Open
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Fix-TLOG-7-Remove-reflection-warnings.patch    
Waiting On: Trevor Bernard

 Description   

A few of my projects are still reflecting using tools.logging 0.2.6 and slf4j-log4j12 "1.7.2".

lein check produces the following output:

...
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isTraceEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isDebugEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isInfoEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isWarnEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isErrorEnabled can't be resolved.
Reflection warning, curator_clj/zookeeper.clj:1:1 - reference to field isErrorEnabled can't be resolved.
...



 Comments   
Comment by Alexander Taggart [ 30/Apr/13 5:37 PM ]

I've made a quick test project: https://github.com/ataggart/reflect-log-test

Cloning that repo, running lein check yields no reflection warnings.

Please provide a way for me to reproduce what you see.

Comment by Trevor Bernard [ 30/Apr/13 5:46 PM ]

Forking that project and will try to find a simple test case that reproduces it.





[TLOG-11] tools.logging doesn't AOT Created: 22/Mar/13  Updated: 30/Apr/13  Resolved: 30/Apr/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Allen Rohner Assignee: Alexander Taggart
Resolution: Declined Votes: 0
Labels: None

Waiting On: Allen Rohner

 Description   

tools.logging does not work if used in a project that is AOT'd.

To reproduce, in a project using tools.logging:

lein jar
lein run

I can workaround it in my project by including

(when *compile-files*
  (compile 'clojure.tools.logging)
  (compile 'clojure.tools.logging.impl))


 Comments   
Comment by Alexander Taggart [ 30/Apr/13 5:11 PM ]

It's not clear to me what "does not work" means.

I've made a quick test project: https://github.com/ataggart/aot-log-test

Cloning that repo, running lein jar then lein run yields the output I would expect.

Further details appreciated.

Comment by Allen Rohner [ 30/Apr/13 6:32 PM ]

This isn't a bug; User Error.

I thought I had fully isolated the bug in my app's codebase, but I forgot about an :injection that we have that jams a debug macro into clojure.core.

for posterity, it was caused by doing

(binding [*ns* (find-ns 'clojure.core)]
  (require 'clojure.tools.logging)
  (defmacro inspect
    "prints the expression '<name> is <value>', and returns the value"
    [value]
    `(do
       (let [value# (quote ~value)
             result# ~value]
         (println value# "is" (with-out-str (clojure.pprint/pprint result#)))
         (clojure.tools.logging/infof "%s is %s" value# result#)
         result#))

Moving the require call to inside the macroexpansion resolved the bug. (And yes, I'm moving to https://github.com/dgrnbrg/spyscope now that I'm aware of it)

Sorry to waste your time, Alex.

Comment by Alexander Taggart [ 30/Apr/13 7:06 PM ]

No worries.

On a related note, do you see any utility in keeping the spy macro? It's pretty much what you're doing. Do you use inspect for logging or as an in-repl debugging tool? Perhaps spy makes more sense in tools.trace.

Comment by Allen Rohner [ 30/Apr/13 7:12 PM ]

TBH, I didn't know about spy. Also, my inspect macro predates clojure.tools.logging, and might even predate clojure.contrib.logging, if you can believe that.

I use inspect as a debugging tool. The main value to jamming it into clojure.core is that I can use it in any file, without having to stop and muck with the ns declaration. Therefore if I was going to use anything else, it would need to have that property.





[TLOG-10] Double-evaluation of arguments Created: 04/Feb/13  Updated: 04/Feb/13  Resolved: 04/Feb/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chris Perkins Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None
Environment:

tools.logging 0.2.5-SNAPSHOT


Attachments: File double-eval.diff    

 Description   

The expansion of the log macros can double-evaluate the first argument.

For example, the following call:

(log/info (expensive-call foo) bar)

Expands to this:

(let*
[logger_278auto_
(get-logger logger-factory #<Namespace scratch.core>)]
(if (enabled? logger_278auto_ :info)
(if (instance? java.lang.Throwable (expensive-call foo))
(log*
logger_278auto_
:info
(expensive-call foo)
(print-str bar))
(log*
logger_278auto_
:info
nil
(print-str (expensive-call foo) bar)))))

Note the calls to "expensive-call" both in the instance check and in the log* call.



 Comments   
Comment by Alexander Taggart [ 04/Feb/13 10:28 PM ]

Fixed in 0.2.6.





[TLOG-9] spy macro (or new spy macro) that accepts a message Created: 08/Dec/12  Updated: 08/Dec/12

Status: Open
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Dave Sann Assignee: Alexander Taggart
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I think it might be useful to have spy be able to take a message to be logged in addition to the data. (or have a similar macro showing an optional message instead of the code).

(spy "doing x" (blah))
(spy :doing-x (blah) ; because it is easier to type sometimes

with spy this may conflict with the log-level.

I find this sort of function with a message useful and use it quite often. Much more often than plain logging. Generally - I would't log the code unless it logged file and line number too.






[TLOG-8] README.md still refers to using version 0.2.3 of tools.logging Created: 24/Nov/12  Updated: 04/Feb/13  Resolved: 04/Feb/13

Status: Closed
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TLOG-8.patch    

 Description   

The latest version is 0.2.4 and should reflected in the README



 Comments   
Comment by Alexander Taggart [ 04/Feb/13 10:27 PM ]

Fixed in 0.2.6.





[TLOG-7] Reflection warnings Created: 16/Apr/12  Updated: 24/Apr/13  Resolved: 05/May/12

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sean Corfield Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-TLOG-7-Remove-reflection-warnings.patch     Text File TLOG-7.1.patch    

 Description   

Reflection warning, clojure/tools/logging.clj:270 - reference to field isTraceEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isDebugEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isInfoEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isWarnEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isErrorEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - reference to field isFatalEnabled can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to trace can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to debug can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to info can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to warn can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to error can't be resolved.
Reflection warning, clojure/tools/logging.clj:270 - call to fatal can't be resolved.



 Comments   
Comment by Alexander Taggart [ 05/May/12 3:26 PM ]

Adds type-hints to all implementation-wrapping fns.

Comment by Trevor Bernard [ 23/Apr/13 4:39 PM ]

A few of my projects are still reflecting using tools.logging 0.2.6 and slf4j-log4j12 "1.7.2". I've attached a patch that I've started using to fix this issue.

Comment by Trevor Bernard [ 23/Apr/13 4:41 PM ]

I'm a registered contributor to clojure.

http://clojure.org/contributing

Comment by Andy Fingerhut [ 24/Apr/13 12:33 PM ]

Trevor, since this ticket was already closed and marked as completed some time ago, it is probably best to create a new ticket and attach your patch to that. I know the issue is similar to this existing ticket, but best to have new tickets for new work.





[TLOG-6] tools.logging does not log to stdout when run from swank-clojure Created: 07/Feb/12  Updated: 07/Feb/12  Resolved: 07/Feb/12

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Trevor Bernard Assignee: Alexander Taggart
Resolution: Declined Votes: 0
Labels: None
Environment:

tools.logging "0.2.3", Mac OS X Lion, emacs 23.4, swank-clojure 1.3.4, slime 20100404



 Description   

Trying to evaluate (info "Hello, World!") doesn't print to stdout when evaluating from within emacs. It works if I `lein run` however.



 Comments   
Comment by Trevor Bernard [ 07/Feb/12 8:44 PM ]

It works when I use the inferior-lisp process but not in slime

Comment by Alexander Taggart [ 07/Feb/12 9:07 PM ]

Configuring swank/slime to work with wherever your log implementation writes its output is not a tools.logging issue.





[TLOG-5] Improvement of sample code in README.md Created: 14/Oct/11  Updated: 17/Oct/11  Resolved: 17/Oct/11

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jingguo Yao Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None


 Description   

README.md uses the following code to show the usage of tools.logging.

(ns example.core
  (:use [clojure.tools.logging :only (info error)]))

(defn divide [one two]
  (try
    (info "Calculated" (/ one two))
    (catch Exception ex
      (error ex "There was an error in calculation"))))

If log4j.rootLogger in the given log4j.properties is updated to "ERROR, A1". "(divide 10 0)" won't evaluate "(error ex "There was an error in calculation")". The reason is that INFO logging level is lower than ERROR logging level. So the info macro will not evaluate "(/ one two)" since the specific INFO logging level is not in effect.

New tools.logging users often expect "(divide 10 0)" to produce logging with ERROR level. They are confused with the above behaviour. So I suggest to make some change to the sample code. Maybe the following code:

(ns example.core
  (:use [clojure.tools.logging :only (info error)]))

(defn divide [one two]
  (try 
    (info "Calculating" one two)
    (/ one two)
    (catch Exception ex
      (error ex "There was an error in calculation"))))





[TLOG-4] Provided implementations of logging protocols fail. Created: 29/Sep/11  Updated: 29/Sep/11  Resolved: 29/Sep/11

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Blocker
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File fix-broken-extend.patch    
Patch: Code and Test

 Description   

Example of error when trying to call a Logger protocol function:

java.lang.IllegalArgumentException: No implementation of method: :enabled? of protocol:
#'clojure.tools.logging.impl/Logger found for class: org.apache.log4j.Logger

The bug is the result of combining syntax-quoting with extend-type, e.g.:

`(do
   (extend-type org.apache.commons.logging.Log
     Logger
     (enabled? [logger# level#] ... )))

The syntax-quote turns the above enabled? into clojure.tools.logging/enabled? protocol function name, and then the extend-type turns that into the keyword :clojure.tools.logging/enabled?, which does not match the expected :enabled?.

Attached patch fixes this by using extend maps rather than extend-type. Also adds some tests.



 Comments   
Comment by Alexander Taggart [ 29/Sep/11 3:36 PM ]

Committed:

https://github.com/clojure/tools.logging/commit/f9cb0a7232d3a11cb7e74e4c13c3adc054e7e7a2





[TLOG-3] Move implementation-specific details out of main ns Created: 06/Jul/11  Updated: 07/Jul/11  Resolved: 07/Jul/11

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File refactor-namespaces.patch    
Patch: Code and Test
Approval: Ok

 Description   

The protocols which allow the main functions/macros to adapt to implementation specific logging libraries do not need to be in the clojure.tools.logging namespace since they are not intended to be used by consumers.

Plan:

  • Move protocols into clojure.tools.logging.impl
  • Move implementation-specific code (e.g., the extentions to the protocols) into separate namespaces, e.g., clojure.tools.logging.log4j.


 Comments   
Comment by Alexander Taggart [ 07/Jul/11 6:49 PM ]

https://github.com/clojure/tools.logging/commit/42dfba84a373e52a4e80af6467e47761e6f20678





[TLOG-2] Change "log" semantics to "logger" semantics Created: 05/Jul/11  Updated: 06/Jul/11  Resolved: 06/Jul/11

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Laurent Petit Assignee: Laurent Petit
Resolution: Completed Votes: 0
Labels: None

Attachments: File tools.logging-TLOG-2-v2.diff    
Patch: Code and Test
Approval: Ok

 Description   

The relevant patch is named tools.logging-TLOG-2-v2.diff

Changes "log" semantics to "logger" semantics.

Log protocol becomes LoggerProtocol
LogFactory protocol becomes LoggerFactoryProtocol
log-factory becomes logger-factory
impl-get-log becomes impl-get-logger
log function argument name becomes logger function argument name
log# macro autosym becomes logger# macro autosym



 Comments   
Comment by Alexander Taggart [ 06/Jul/11 3:00 PM ]

Committed to master:
https://github.com/clojure/tools.logging/commit/0a5f9bd3148cf078dd0b62aece5f92a970b5aba3





[TLOG-1] Non-optimized, no-exception path for logp calls log* with wrong arity Created: 01/Jun/11  Updated: 01/Jun/11  Resolved: 01/Jun/11

Status: Resolved
Project: tools.logging
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TLOG-1.patch    
Patch: Code and Test

 Description   

Per Laurent Petit



 Comments   
Comment by Alexander Taggart [ 01/Jun/11 11:34 AM ]

https://github.com/clojure/tools.logging/commit/687c1833c59ad1204c2a41cf0ed1f53be4a0fa8d





[TGEN-4] Typo in defspec docstring Created: 11/Mar/13  Updated: 11/Mar/13

Status: Open
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Ambrose Bonnaire-Sergeant Assignee: Stuart Halloway
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The docstring for `defspec` misspells "metdata".






[TGEN-3] binding *seed* does not change the basis Created: 13/Oct/12  Updated: 03/Feb/13  Resolved: 03/Feb/13

Status: Closed
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Stuart Halloway
Resolution: Completed Votes: 0
Labels: None


 Description   

This is what I'm observing:

user=> (binding [gen/*seed* 7] (gen/uniform))
-6880819372607069048
user=> (binding [gen/*seed* 7] (gen/uniform))
5065957067589062886

This is what I'm expecting:

user=> (binding [gen/*seed* 7] (gen/uniform))
-4967725919621401576
user=> (binding [gen/*seed* 7] (gen/uniform))
-4967725919621401576

The work-around is to bind *rnd* to a new java.util.Random constructed appropriately:

(binding [gen/*rnd* (java.util.Random. 7)] (gen/uniform))

I'm not sure: Is this is a bug in the doc string of *seed*? Is the bug the very existence of *seed*? (*seed* is never used.) Should the doc-string for *rnd* suggest something like the code above as a way to get a consistent basis? Should generators provide a with-seed macro to do this for us?



 Comments   
Comment by Stuart Halloway [ 03/Feb/13 7:13 AM ]

this is fixed on master in the data.generators project





[TGEN-2] ASCII DEL (0x7f, 127) is not a printable character Created: 13/Oct/12  Updated: 14/Oct/12  Resolved: 14/Oct/12

Status: Resolved
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Ben Smith-Mannschott
Resolution: Completed Votes: 0
Labels: None

Attachments: File ascii-127-is-not-printable.diff    
Patch: Code
Approval: Accepted

 Description   

ASCII 127 is a control character.

Dec Hex    Dec Hex    Dec Hex  Dec Hex  Dec Hex  Dec Hex   Dec Hex   Dec Hex  
  0 00 NUL  16 10 DLE  
  1 01 SOH  17 11 DC1  
  2 02 STX  18 12 DC2  
  3 03 ETX  19 13 DC3  
  4 04 EOT  20 14 DC4  
  5 05 ENQ  21 15 NAK  
  6 06 ACK  22 16 SYN  
  7 07 BEL  23 17 ETB  
  8 08 BS   24 18 CAN  
  9 09 HT   25 19 EM   
 10 0A LF   26 1A SUB  
 11 0B VT   27 1B ESC  
 12 0C FF   28 1C FS   
 13 0D CR   29 1D GS   
 14 0E SO   30 1E RS   
 15 0F SI   31 1F US                                                 127 7F DEL


 Comments   
Comment by Ben Smith-Mannschott [ 14/Oct/12 11:14 AM ]

Fixed by a73c11e3 on test.generative.





[TGEN-1] generators/shuffle violates contract of Comparable Created: 13/Oct/12  Updated: 14/Oct/12  Resolved: 14/Oct/12

Status: Resolved
Project: test.generative
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ben Smith-Mannschott Assignee: Ben Smith-Mannschott
Resolution: Completed Votes: 0
Labels: None
Environment:

Mac OS X 10.8.2
java version "1.7.0_06"
Java(TM) SE Runtime Environment (build 1.7.0_06-b24)
Java HotSpot(TM) 64-Bit Server VM (build 23.2-b09, mixed mode)


Attachments: File fisher-yates-shuffle.diff    
Patch: Code
Approval: Accepted

 Description   

JDK 7's sort function is stricter about verifying that implementations of the comparison function actually obey their contract. It will even helpfully throw an exception when this is not the case:

java.lang.IllegalArgumentException: Comparison method violates its general contract!

The root cause is this gem:

generators.clj
(defn shuffle
      "Shuffle coll"
      [coll]
      (sort-by (fn [_] (long)) coll))

Which does not work on JDK7 since TimSort is clever enough to detect that the ordering function is behaving inconsistently, resulting in the previously mentioned exception.

Also, this is just a bad idea:

A variant of the above method that has seen some use in languages that support sorting with user-specified comparison functions is to shuffle a list by sorting it with a comparison function that returns random values. However, this is an extremely bad method: it is very likely to produce highly non-uniform distributions, which in addition depends heavily on the sorting algorithm used.

(wikipedia)



 Comments   
Comment by Ben Smith-Mannschott [ 14/Oct/12 11:13 AM ]

Fixed by 5a59bf0f on test.generative.





[TCLI-3] Change contract to provide access to banner on parse error Created: 09/Feb/13  Updated: 08/Mar/13

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Philip Aston Assignee: Gareth Jones
Resolution: Unresolved Votes: 1
Labels: None


 Description   

If a user provides an invalid option, most applications would want to display the banner.

tools.cli throws an Exception with minimal information, and (banner-for) is marked private.

The exception could be replaced with one that provides access to the banner, but I think it would be better (easier to use, more idiomatic?) to change the contract so that the banner is always returned. Given the exception string is also useful, perhaps change cli to return a map with keys [:options :extra-args :banner :parse-failure]?



 Comments   
Comment by Gunnar Völkel [ 08/Mar/13 4:19 AM ]

I agree with the reporter. Throwing an exception for invalid arguments ("[...] is not a valid argument") is not appropriate since you usually want to display that error message along with the banner.





[TCLI-2] Allow caller-supplied parse-fn Created: 11/Nov/12  Updated: 11/Apr/13

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Pierre-Yves Ritschard Assignee: Gareth Jones
Resolution: Unresolved Votes: 0
Labels: enhancement

Attachments: File TCLI-2.diff    

 Description   

As for :parse-fn, a function can be supplied, this
is useful for standard use-cases such as: -vv, or when
you want to build a list from values.

PR: https://github.com/clojure/tools.cli/pull/11



 Comments   
Comment by Andy Fingerhut [ 11/Nov/12 12:38 PM ]

Pierre-Yves, there are instructions for creating patches under the headings "Development" and "Adding patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Submissions to this module do require the author to sign a CA. Instructions here: http://clojure.org/contributing

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Suggested Patch

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Hello, now that I'm a registered contributor, I attached a file as suggested in the workflow wiki





[TCLI-1] Do not include keys when no value provided and no :default Created: 18/May/12  Updated: 24/Aug/12  Resolved: 24/Aug/12

Status: Closed
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCLI-1-0001.patch     Text File TCLI-1-0002.patch    
Patch: Code and Test

 Description   

I was trying to use tools.cli in conjunction with a configuration map
loaded from a file, and use clojure.core/merge to combine the results.

This didn't work because tools.cli always uses a default value of
nil, even when no default value is specified. The nil always
overrides defaults from another source.

Example before the patch:

(def my-defaults
  {:foo 1})

(merge my-defaults
       (first (clojure.tools.cli/cli
               [] ; no arguments given
               ["--foo"]))) ; no default specified
;;=> {:foo nil}

This enhancement modifies tools.cli to completely omit arguments which
have no default specified and no value given on the command line.

After the patch, the above example returns:

;;=> {:foo 1}


 Comments   
Comment by Stuart Sierra [ 20/Jul/12 7:41 AM ]

New patch file 0002 fixes syntax error in the previous patch; updated description to show correct results.

Comment by Stuart Sierra [ 24/Aug/12 8:31 AM ]

Patch applied in commit https://github.com/clojure/tools.cli/commit/f9d92395cb788dd08cb144035a9d5fd8706d10b5





[TBENCH-16] Add performance testing for STM Created: 23/Feb/12  Updated: 23/Feb/12

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stefan Kamphausen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance, test
Environment:

64bit Linux, Quad-core


Attachments: File stm-perf-TBENCH-16.diff    

 Description   

Write a test-suite to detect regressions in STM performance.



 Comments   
Comment by Stefan Kamphausen [ 23/Feb/12 2:38 PM ]

Patch which adds an stm-namespace containing a collection of performance test functions.





[TBENCH-15] Alioth nbody Created: 21/Jan/12  Updated: 22/Jan/12  Resolved: 22/Jan/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 22/Jan/12 12:36 PM ]

Done, https://github.com/clojure/test.benchmark/commit/b0772ea5858a3924b15d87003cb039af20c0cfa7





[TBENCH-14] Alioth fannkuch-redux Created: 21/Jan/12  Updated: 22/Jan/12

Status: In Progress
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None





[TBENCH-13] Alioth pidgits Created: 21/Jan/12  Updated: 21/Jan/12  Resolved: 21/Jan/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 21/Jan/12 1:08 PM ]

TBENCH-4





[TBENCH-12] Alioth k-nucleotide Created: 21/Jan/12  Updated: 23/Feb/12

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Comments   
Comment by Mike Anderson [ 21/Feb/12 5:00 AM ]

Have added my latest version of the k-nucleotide benchmark in the following repository:

https://github.com/mikera/test.benchmark/

Comment by David Nolen [ 21/Feb/12 8:00 AM ]

Please create a patch of your changes and add it to the ticket, thanks!

Comment by Paul Michael Bauer [ 23/Feb/12 3:43 PM ]

If you add a patch, I can take a look at this tonight.

Comment by Paul Michael Bauer [ 23/Feb/12 10:45 PM ]

Mike,
I did pull down your repo and ran some tests.
This version of k-nucleotide does run faster than the currently submitted version in alioth.
However, we've got a ways to go to catch the faster Java implementations.

I've added a few comments in your repo.
Of note, the (set! unchecked-math true) directive at the top of files allows us to use inc, dec, etc directly without explicitly specifying unchecked-inc, etc.

http://shootout.alioth.debian.org/u64q/program.php?test=knucleotide&lang=java&id=9 is the fastest java version, but is a bit gnarly.
http://shootout.alioth.debian.org/u64q/program.php?test=knucleotide&lang=java&id=2 is a little slower, but in the same order of magnitude for performance, and significantly more straightforward.

You might check those out for some inspiration to boost the performance of clojure's knucleotide.
Good luck and thanks for the help!





[TBENCH-11] Alioth reverse-complement Created: 21/Jan/12  Updated: 13/Feb/12

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Paul Michael Bauer
Resolution: Unresolved Votes: 0
Labels: None





[TBENCH-10] Alioth spectral-norm Created: 21/Jan/12  Updated: 21/Jan/12  Resolved: 21/Jan/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 21/Jan/12 1:02 PM ]

Fixed, https://github.com/clojure/test.benchmark/commit/642279639c99ce222c10a4d1f1f69aa5565845c6





[TBENCH-9] Alioth regex-dna Created: 15/Nov/11  Updated: 13/Feb/12  Resolved: 13/Feb/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Paul Michael Bauer Assignee: Paul Michael Bauer
Resolution: Completed Votes: 0
Labels: None

Patch: Code and Test

 Description   

Implement regex-dna as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Paul Michael Bauer [ 13/Feb/12 11:11 PM ]

re-worked A. Fingerhut's implementation to use nio and a hand-rolled reduce loop for code replacement.
Result is a ~2x speedup and 92% as fast as the fastest Java version.

https://github.com/clojure/test.benchmark/blob/0977c41e430b85f452f0064aad24562e9be48fff/src/main/clojure/alioth/regexdna.clj





[TBENCH-8] Alioth mandelbrot Created: 13/Sep/11  Updated: 15/Nov/11  Resolved: 15/Nov/11

Status: Closed
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Paul Michael Bauer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: File mandelbrotR1.diff    
Patch: Code and Test
Approval: Ok

 Description   

Implement mandelbrot as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Paul Michael Bauer [ 13/Sep/11 6:06 AM ]

This is a first pass at a mandelbrot that takes advantage of clojure 1.3 performance features.

Approach:
Avoid idiomatic functional code.
Extensive iteration, in-place mutation, primitive arrays, manage our own threads etc. Not "pretty."
(currently .8 to .9 % of fastest java version).

Tests:
Correctness tests, verified against a set of known-good resource files.

Caveats:
This is the first pass and my first time writing optimized clojure 1.3. I'm sure there are things that can be improved, so any feedback welcome.

Future:
I've included a modified version of the fastest java program in the patch as I've found it useful for comparison
(# time script/run_java alioth.java.mandelbrot 16000)
One thing we don't have is repeatable performance metrics/tests.
One approach would be to have tests that fail whenever the clojure version falls below a certain percentage of the java version.
That's something I'm working on.

Comment by Paul Michael Bauer [ 13/Sep/11 9:00 PM ]

Per discussion on mailing list (https://groups.google.com/d/topic/clojure-dev/Kr5teQh69Pw/discussion) removed java files.
Files pushed.

Comment by Stuart Halloway [ 15/Nov/11 7:29 PM ]

Paul, it is fine for you to close tickets yourself on contrib projects (all the notes about "core team only" apply to Clojure itself, not the contribs).





[TBENCH-7] Auto-generate maven-classpath on build, script/* refactoring Created: 10/Sep/11  Updated: 15/Nov/11  Resolved: 15/Nov/11

Status: Closed
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Paul Michael Bauer Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Refactored-script-run-repl-auto-generate-script-mave.patch    
Patch: Code
Waiting On: Stuart Halloway

 Description   
  • Update pom to auto-generate maven-classpath file on build
  • script refactoring, can run from any folder, misc improvements
  • Update pom to depend on 1.3 beta 3


 Comments   
Comment by Paul Michael Bauer [ 13/Sep/11 9:01 PM ]

This can be closed. Patch pushed.





[TBENCH-6] thread-ring Created: 29/Oct/10  Updated: 15/Nov/11  Resolved: 15/Nov/11

Status: Closed
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Paul Michael Bauer
Resolution: Completed Votes: 0
Labels: None

Patch: Code and Test

 Description   

Implement thread-ring as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident

Comment by Paul Michael Bauer [ 07/Oct/11 1:45 PM ]

Could you assign this to me ... working on it.

Comment by Paul Michael Bauer [ 15/Nov/11 7:36 PM ]

Fixed:
https://github.com/clojure/test.benchmark/commit/f4c0802af58f5f3341816be40bf1388029bc0d3e
https://github.com/clojure/test.benchmark/commit/f4c0802af58f5f3341816be40bf1388029bc0d3e

Comment by Paul Michael Bauer [ 15/Nov/11 7:36 PM ]

https://github.com/clojure/test.benchmark/commit/f4c0802af58f5f3341816be40bf1388029bc0d3e





[TBENCH-5] spectral-norm Created: 29/Oct/10  Updated: 21/Jan/12  Resolved: 21/Jan/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Implement spectral-norm as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident

Comment by David Nolen [ 21/Jan/12 1:07 PM ]

Done, https://github.com/clojure/test.benchmark/commit/642279639c99ce222c10a4d1f1f69aa5565845c6





[TBENCH-4] Alioth pidigits Created: 29/Oct/10  Updated: 03/Dec/10

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Implement pidigits as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident





[TBENCH-3] Alioth meteor-contest Created: 29/Oct/10  Updated: 03/Dec/10

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Implement meteor-contest as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident





[TBENCH-2] Alioth chameneos-redux Created: 29/Oct/10  Updated: 03/Dec/10

Status: Open
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Implement chameneos-redux as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident





[TBENCH-1] Alioth binary-trees Created: 29/Oct/10  Updated: 21/Jan/12  Resolved: 21/Jan/12

Status: Resolved
Project: test.benchmark
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Description   

Implement binary-trees as described at http://shootout.alioth.debian.org/. See http://dev.clojure.org/display/testbenchmark/Alioth+Benchmarks for an overview.



 Comments   
Comment by Stuart Halloway [ 03/Dec/10 11:46 AM ]

these were assigned to me by accident

Comment by David Nolen [ 21/Jan/12 11:22 AM ]

done as far I can tell.





[NREPL-41] Rebind print-.* dynamic vars when loading a file Created: 21/Mar/13  Updated: 21/Mar/13  Resolved: 21/Mar/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Colin Jones
Resolution: Completed Votes: 0
Labels: None


 Description   

Syntax errors happen when the user has bound things like *print-length*, *print-depth* and those take precedence.

See https://github.com/technomancy/leiningen/issues/952

Based on discussion in that ticket, I think the right approach is to rebind these vars when doing pr-str in load_file.clj - but I haven't yet been able to reproduce locally yet.



 Comments   
Comment by Colin Jones [ 21/Mar/13 11:38 PM ]

fixed on master: https://github.com/clojure/tools.nrepl/commit/11e3993746950640cb8173fbcd3b074c53b26ce0





[NREPL-40] Thread leak in clojure.tools.nrepl.transport$fn_transport? Created: 11/Mar/13  Updated: 12/Mar/13

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.1, 0.2.2
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Lao Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Windows 7 x64, Oracle JDK 1.7.0.11 x64, clojure 1.4.0



 Description   

When trying out remote eval using your sample in the README, ie

(with-open [conn (repl/connect :port 59258)]
(-> (repl/client conn 1000) ; message receive timeout required
(repl/message {:op "eval" :code "(+ 2 3)"})
repl/response-values))

I'm noticing that hosting process leaking a thread each time the remote eval is called. Jconsole shows a clojure-agent-send-off-pool-xxx thread got spawn as result of the call. The stack appears to be pointing to the "while true" loop inside fn_transport.

java.util.concurrent.locks.LockSupport.park(LockSupport.java:186)
java.util.concurrent.SynchronousQueue$TransferStack.awaitFulfill(SynchronousQueue.java:458)
java.util.concurrent.SynchronousQueue$TransferStack.transfer(SynchronousQueue.java:359)
java.util.concurrent.SynchronousQueue.put(SynchronousQueue.java:878)
clojure.tools.nrepl.transport$fn_transport$fn__3912.invoke(transport.clj:44)
clojure.core$binding_conveyor_fn$fn__3989.invoke(core.clj:1819)
clojure.lang.AFn.call(AFn.java:18)
java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:334)
java.util.concurrent.FutureTask.run(FutureTask.java:166)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
java.lang.Thread.run(Thread.java:722)

What do you recommend as way to free the thread? I have server code that calls nrepl on behave of client connections and the number of call can pile up fairly quickly.



 Comments   
Comment by David Lao [ 12/Mar/13 3:37 PM ]

Here is my workaround.

— a/src/main/clojure/clojure/tools/nrepl/transport.clj
+++ b/src/main/clojure/clojure/tools/nrepl/transport.clj
@@ -36,12 +36,12 @@
to the 2 or 3 functions provided."
([read write] (fn-transport read write nil))
([read write close]

  • (let [read-queue (SynchronousQueue.)]
  • (future (try
  • (while true
  • (.put read-queue (read)))
  • (catch Throwable t
  • (.put read-queue t))))
    + (let [read-queue (SynchronousQueue.)
    + transport-thread (future (try
    + (while true
    + (.put read-queue (read)))
    + (catch Throwable t
    + (.put read-queue t))))]
    (FnTransport.
    (let [failure (atom nil)]
    #(if @failure
    @@ -51,7 +51,7 @@
    (do (reset! failure msg) (throw msg))
    msg))))
    write
  • close))))
    + (fn [] (close)(future-cancel transport-thread))))))




[NREPL-39] Using functions reading from *in* causes "java.io.IOException: Write end dead" Created: 03/Feb/13  Updated: 07/Apr/13  Resolved: 21/Mar/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Adrian Bendel Assignee: Colin Jones
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Use-a-queue-for-stdin-to-avoid-PipedReader-Writer.patch    

 Description   

When calling (read-line) from lein repl or ccw repl, waiting around four minutes (on my machine),
evaluating any form causes the following exception:

ERROR: Unhandled REPL handler exception processing message {:code (+ 1 1), :id 08e30b70-ab0f-4a47-8937-3c040ce06f4f, :ns user, :op eval, :session 916200cc-e476-4ac4-98f6-76057a9020be}
java.io.IOException: Write end dead
	at java.io.PipedReader.ready(Unknown Source)
	at clojure.tools.nrepl.middleware.session.proxy$java.io.PipedReader$0.ready(Unknown Source)
	at java.io.BufferedReader.ready(Unknown Source)
	at java.io.FilterReader.ready(Unknown Source)
	at java.io.PushbackReader.ready(Unknown Source)
	at clojure.tools.nrepl.middleware.session$add_stdin$fn__1103.invoke(session.clj:197)
	at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__840.invoke(middleware.clj:17)
	at clojure.tools.nrepl.middleware.session$session$fn__1093.invoke(session.clj:165)
	at clojure.tools.nrepl.middleware$wrap_conj_descriptor$fn__840.invoke(middleware.clj:17)
	at clojure.tools.nrepl.server$handle_STAR_.invoke(server.clj:18)
	at clojure.tools.nrepl.server$handle$fn__1132.invoke(server.clj:27)
	at clojure.core$binding_conveyor_fn$fn__4130.invoke(core.clj:1836)
	at clojure.lang.AFn.call(AFn.java:18)
	at java.util.concurrent.FutureTask$Sync.innerRun(Unknown Source)
	at java.util.concurrent.FutureTask.run(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
	at java.lang.Thread.run(Unknown Source)

While googling a bit on this and without concrete knowledge of the implementation in nrepl,
it seems like closing a thread that wrote to PipedWriter, before closing the writer or writing to it from another thread,
causes PipedReader#ready to throw that exception.



 Comments   
Comment by Colin Jones [ 06/Mar/13 5:46 PM ]

I found this related article: http://techtavern.wordpress.com/2008/07/16/whats-this-ioexception-write-end-dead/

So it seems like we may need something a little more robust. I'm thinking just a LinkedBlockingQueue that gets written to on input, with a Reader proxy over the top to hook up to Clojure's LineNumberingPushbackReader. I've started fiddling with that idea, passing most of the tests already, and I think this is the right path to go down in general. Yell at me if you see a problem with this plan.

Comment by Chas Emerick [ 07/Mar/13 4:50 AM ]

I had no idea that PipedReader & co. actually track the threads that touch them. That makes them entirely inappropriate for the *in* case.

+1 to your plan, thanks for chasing this!

Comment by Colin Jones [ 08/Mar/13 3:28 PM ]

OK, this works for me against REPLy.

I'd have preferred to avoid the dynamic binding (skipping-eol), but I haven't been able to think of another way to accomplish what it does. The problem is that we don't want to send another :need-input message just to clear newlines, but we don't have any visibility into what's in the LineNumberingPushbackReader's buffer (neither the full deal, nor the pushback). My initial thought was to replace the LineNumberingPushbackReader directly, with a custom thing, but I think the assumptions around that class in `read` and `read-line` are too concretely baked in to allow it (especially before 1.5, when the buffer size became configurable).

And there's no room for adding bits to the Reader interface, so in order to effect a behavior difference in the Reader, depending on what the high-level state is, I don't know that there are any options aside from dynamic binding or an atom (and of the two, a var seemed more robust, no worries about finally & friends).

Comment by Colin Jones [ 21/Mar/13 11:39 PM ]

Pushed to master: https://github.com/clojure/tools.nrepl/commit/3cd3d0e8b2091fc70fde1587d3f0a995cbf8b283





[NREPL-38] Certain Calendar values don't seem to be able to print Created: 12/Jan/13  Updated: 13/Jan/13  Resolved: 13/Jan/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.1

Type: Defect Priority: Major
Reporter: Julian Birch Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bug
Environment:

Ubuntu 12.10, running through Leiningen 2



 Description   

If I try to run lein repl outside of a project folder, I get Clojure 1.4.0.
I then run

(println (javax.xml.bind.DatatypeConverter/parseDateTime "2008-07-21T19:17:29"))

which produces

IndexOutOfBoundsException start 26, end 2, s.length() 28 java.lang.AbstractStringBuilder.append (AbstractStringBuilder.java:476)
#inst "2008-07-21T19:17:29.000+01:

Note the absence of a closing '"'.

My apologies in advance if this is impossible to reproduce.



 Comments   
Comment by Andy Fingerhut [ 13/Jan/13 9:55 AM ]

I can reproduce this with Clojure 1.4.0 and 1.5.0-RC2 if I do it within "lein2 repl" (I was using Leiningen version 2.0.0-preview10 to reproduce the problem).

If I use "java -cp clojure.jar clojure.main" to start a REPL session, with either Clojure 1.4.0 or 1.5.0-RC2 for clojure.jar, I don't see any problem. I was testing on Mac OS X 10.6.8 with Oracle/Apple JDK 1.6.0_37.

I also don't see this problem if I use Leiningen version 1.7.1, tested with both Clojure 1.4.0 and 1.5.0-RC2.

This appears to be some kind of bad interaction between Leiningen 2.0.0-preview10 and Clojure.

Comment by Andy Fingerhut [ 13/Jan/13 10:05 AM ]

I also reproduced this issue with the latest version of Leiningen, which is 2.0.0-RC2. Email sent to the Leiningen developer email list so they know about it.

Comment by Chas Emerick [ 13/Jan/13 10:52 AM ]

This is an nREPL bug involving an API mismatch between java.io.Writer.write() and java.lang.AbstractStringBuilder.append().

The fix is simple; patch release coming later today.

Comment by Chas Emerick [ 13/Jan/13 12:29 PM ]

Fixed with b9e930a1.

Will be a part of [org.clojure/tools.nrepl "0.2.1"], to be released later today.





[NREPL-37] Printing reference returned by clojure.tools.nrepl.server/start-server causes multimethod exception Created: 20/Dec/12  Updated: 26/Feb/13  Resolved: 26/Feb/13

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-RC1
Fix Version/s: 0.2.2

Type: Defect Priority: Minor
Reporter: Vaughn Dickson Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug


 Description   

I was accidentally printing the reference returned by start-server by calling it as the final function in my main method, which causes this error:

java.lang.IllegalArgumentException: Multiple methods in multimethod 'print-method' match dispatch value: class clojure.tools.nrepl.server.Server -> interface clojure.lang.IDeref and interface clojure.lang.IRecord, and neither is preferred
at clojure.lang.MultiFn.findAndCacheBestMethod(MultiFn.java:136)
at clojure.lang.MultiFn.getMethod(MultiFn.java:111)
at clojure.lang.MultiFn.getFn(MultiFn.java:119)
at clojure.lang.MultiFn.invoke(MultiFn.java:167)
at clojure.core$pr_on.invoke(core.clj:3266)
at clojure.core$pr.invoke(core.clj:3278)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:601)
at clojure.core$prn.doInvoke(core.clj:3311)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.main$eval_opt.invoke(main.clj:299)
at clojure.main$initialize.invoke(main.clj:316)
at clojure.main$null_opt.invoke(main.clj:349)
at clojure.main$main.doInvoke(main.clj:427)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:419)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:532)
at clojure.main.main(main.java:37)



 Comments   
Comment by Chas Emerick [ 02/Feb/13 7:35 AM ]

The fact that the returned record is also a clojure.lang.IDeref is a temporary compatibility measure, a result of changing to returning a record.

Adding a print-method implementation for the Server type that explicitly delegates to the IRecord implementation would resolve the problem.

Comment by Chas Emerick [ 26/Feb/13 4:42 AM ]

Fixed @ http://github.com/clojure/tools.nrepl/commit/0f016eb





[NREPL-36] Too many DynamicClassLoaders created Created: 10/Dec/12  Updated: 10/Dec/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-RC1, 0.2.0-beta9, 0.2.0-beta10
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Not sure whether this ticket belongs here or in the Clojure-proper JIRA, so feel free to close if this is an inappropriate location. clojure.main/repl creates a new DynamicClassLoader on every execution, so it looks like the stack of classloaders grows without bounds. Seems a bit similar to http://dev.clojure.org/jira/browse/NREPL-31 in that clojure.main/repl has another assumption about when clojure.main/repl will run.

See https://groups.google.com/forum/?fromgroups=#!topic/clojure/firG9zTVecU%5B1-25%5D for the original report.






[NREPL-35] Eliminate several uses of reflection in tools.nrepl Created: 28/Oct/12  Updated: 31/Oct/12  Resolved: 31/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File nrepl-35-eliminate-reflection-v1.txt    

 Description   

There are several uses of reflection in tools.nrepl that can be eliminated with suitable type hints.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 10:12 PM ]

nrepl-35-eliminate-reflection-v1.txt dated Oct 28 2012 adds type hints to eliminate some of the reflection warnings in tools.nrepl. There are still some left over that are not as obvious how to eliminate, if it is even possible.

Comment by Chas Emerick [ 31/Oct/12 4:44 AM ]

Thanks for the patch, but it overhinted in some areas (I try to keep hints at an absolute minimum to maximize readability) and overconstrained in others (e.g. ThreadPoolExecutor vs. Executor).

Fixed @ https://github.com/clojure/tools.nrepl/commit/ac95dc7da973f3d7edea75d0f6f9ce01ee18641d





[NREPL-34] Default load-file middleware cannot load large files due to JVM method size limitations Created: 11/Oct/12  Updated: 11/Oct/12  Resolved: 11/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta10
Fix Version/s: 0.2.0-RC1

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Simply interpolating contents of files to be loaded into a Compiler/load expression to be evaluated does not work with very large files. (See http://code.google.com/p/counterclockwise/issues/detail?id=429)



 Comments   
Comment by Chas Emerick [ 11/Oct/12 11:34 PM ]

Fixed in https://github.com/clojure/tools.nrepl/commit/77cb7d2347b0a78094ea32d7757aa3e3662fc634





[NREPL-33] Consider making session and eval functionality more accessible Created: 08/Oct/12  Updated: 08/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

ritz re-uses some of nrepl's private functions to avoid duplication. The uses are listed below.

Would it be possible to make these functions public? More subjectively, it might also be worth considering factoring out the session functionality into it's own namespace (including msg and possibly queue-eval), so the functionality is not split across the session middleware and the interruptible-eval middleware.

The debug nrepl server:
https://github.com/pallet/ritz/blob/develop/nrepl/src/ritz/nrepl.clj#L189

This uses clojure.tools.nrepl.middleware.session/create-session and clojure.tools.nrepl.middleware.session/session-out

ritz provides an eval op that tracks source forms:
https://github.com/pallet/ritz/blob/develop/nrepl-middleware/src/ritz/nrepl/middleware/tracking_eval.clj

This uses clojure.tools.nrepl.middleware.interruptible-eval/queue-eval and clojure.tools.nrepl.middleware.interruptible-eval/configure-executor






[NREPL-32] Handling of messages is serialized on a per-connection basis Created: 03/Oct/12  Updated: 04/Oct/12  Resolved: 04/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta10

Type: Defect Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 04/Oct/12 12:31 AM ]

Fixed @ 2f12262834e5b93b8536c81b12df25b2b75e0254.





[NREPL-31] REPL utilities are refered into *ns* prior to every expression evaluation Created: 01/Oct/12  Updated: 22/Oct/12  Resolved: 22/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9
Fix Version/s: 0.2.0, 0.2.0-RC1

Type: Defect Priority: Critical
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Declined Votes: 0
Labels: None


 Description   

The refers of REPL utilities (pst, pprint, etc) happens in clojure.main/repl. In a terminal/single-threaded REPL, this is called just once, so it only ever affects the user namespace. Every expression sent for evaluation by nREPL invokes clojure.main/repl though, so moving *ns* around will inadvertently cause those refers to happen over and over, into non-user namespaces. (I've been enjoying having pprint and pp available all the time, but I'd never thought much about why they were always there.)

In the end, this is an nREPL bug.

I don't see any easy way out off the top of my head. I think nREPL will end up having to stop using clojure.main/repl, and maintain a modified version of it itself (something I wanted to avoid exactly so as to benefit from the changes to clojure.main/repl from version to version of Clojure).

Suggestions most welcome.

(Originally reported here.)



 Comments   
Comment by Chas Emerick [ 11/Oct/12 2:10 PM ]

Hopefully CLJ-1085 is resolved and this can drop off. In the meantime, not going to hold off -beta10 for it or this.

Comment by Chas Emerick [ 22/Oct/12 8:01 PM ]

Fixed upstream in CLJ-1085. Note that this issue will continue to affect those using Clojure < 1.5.0.





[NREPL-30] Investigate hangs when server goes away Created: 14/Sep/12  Updated: 08/Oct/12  Resolved: 08/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9, 0.2.0-beta10
Fix Version/s: 0.2.0-beta10

Type: Task Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

See https://github.com/trptcolin/reply/issues/84#issuecomment-8481753 and maybe http://code.google.com/p/counterclockwise/issues/detail?id=405



 Comments   
Comment by Colin Jones [ 14/Sep/12 9:03 AM ]

If it helps, I did observe in some testing that after the server side of the socket goes down, writing to and flushing the client socket's output stream twice would bubble up a client exception. That seems a little keepalive-ish, but I'm not sure whether that sort of thing would do other transports any good.

Comment by Chas Emerick [ 03/Oct/12 12:37 PM ]

A fix for this is on master now, with commit 8a5dad2045434fcc06f2878de55f7dcdefa01a1b. There were a number of issues in the implementation of FnTransport that were preventing exceptions from bubbling up as they should have been.

No APIs were harmed in the course of implementing a fix. Also, no keepalive mechanism was required (which makes me slightly suspicious that the issue has actually been fixed, but we'll see). (Not TCP keepalive; that's irrelevant to the issue, and wouldn't help even if it were reliably standard/present.)

New tests added, and manual verification against nREPL servers running in different processes looks good too. I'll put out an 0.2.0-beta10 shortly so that people can start banging on this to see if it resolves issues in downstream projects/tools.

Note that I also found that it sometimes requires two sends (messages here, not writes to a socket) to provoke an error when writing to a transport that had been connected to a server that was closed or otherwise went away. Maybe it's a buffering issue? I don't have a good theory. Check out e.g. nrepl-test/transports-fail-on-disconnects if you want to play around with it.

Comment by Chas Emerick [ 04/Oct/12 5:06 PM ]

Note that the bencode transport should throw a java.net.SocketException when its connection is lost.

Comment by Chas Emerick [ 08/Oct/12 12:42 PM ]

Good reports received from downstream projects on the fix for this. Calling it good.





[NREPL-29] Provide a mechanism for overriding an operation Created: 09/Sep/12  Updated: 13/Nov/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0-beta9, 0.2.0-beta10
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

When specifying middleware, it would be much easier for the user to be able to override a default middleware without having to specify a handler.

For example, if there is a default middleware providing the "complete" operation, the user should be able to just specify their preferred completion middleware, without having to specify all middleware as a handler.

One way to do this might be to check metadata for the provided operations of the specified metadata, and ensure that either the default middleware for that operation is removed, or that the specified middleware takes precendence (which may be simpler when a middleware provides multiple operations).



 Comments   
Comment by Chas Emerick [ 14/Sep/12 7:44 AM ]

Agreed.

Just making sure that the order in which additional middlewares are provided is taken as a default stack order will suffice for most use cases. Transforming middlewares (either in full or in part) would need another metadata slot, :replace perhaps, though it seems like that would be much more difficult to get right.





[NREPL-28] Clarify semantics for String encoding Created: 21/Aug/12  Updated: 11/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 21/Aug/12 8:27 PM ]

Initial discussion held in NREPL-11.





[NREPL-26] Middleware metadata + collection of middleware names => well-formed handler Created: 10/Aug/12  Updated: 20/Aug/12  Resolved: 20/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 20/Aug/12 3:48 PM ]

See clojure.tools.nrepl.middleware, the metadata descriptors described there, etc.





[NREPL-25] Metadata -> middleware/handler documentation Created: 10/Aug/12  Updated: 13/Aug/12  Resolved: 13/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Blocker
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by Chas Emerick [ 13/Aug/12 11:31 PM ]

See clojure.tools.nrepl.middleware/wrap-describe, set-descriptor!, etc.





[NREPL-24] :session key is overloaded Created: 10/Aug/12  Updated: 03/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The session middleware takes the :session key from the message, and uses it as an id to lookup session data, which it then places on the same key.

Using a separate key for the session id and the session data would be less confusing, and would allow easier checking of the availability of the session data in the message map.



 Comments   
Comment by Chas Emerick [ 10/Aug/12 11:26 PM ]

The session id will need to continue to come in in the :session slot, as I don't want to break clients (and, there's very, very few middlewares out there yet).

Suggestions for names for the actual session atom? :the-session? :-/





[NREPL-23] NPE when interruptible-eval/evaluate is passed a non-existant namespace Created: 22/Jul/12  Updated: 20/Aug/12  Resolved: 20/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.0-beta9

Type: Defect Priority: Major
Reporter: Joe Snikeris Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   
Stack Trace
Exception in thread "nREPL-worker-2" java.lang.NullPointerException
	at clojure.core$refer.doInvoke(core.clj:3779)
	at clojure.lang.RestFn.applyTo(RestFn.java:139)
	at clojure.core$apply.invoke(core.clj:603)
	at clojure.core$load_lib.doInvoke(core.clj:5279)
	at clojure.lang.RestFn.applyTo(RestFn.java:142)
	at clojure.core$apply.invoke(core.clj:603)
	at clojure.core$load_libs.doInvoke(core.clj:5298)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at clojure.core$apply.invoke(core.clj:605)
	at clojure.core$use.doInvoke(core.clj:5392)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.main$repl.doInvoke(main.clj:259)
	at clojure.lang.RestFn.invoke(RestFn.java:1096)
	at clojure.tools.nrepl.middleware.interruptible_eval$evaluate$fn__337.invoke(interruptible_eval.clj:51)
	at clojure.lang.AFn.applyToHelper(AFn.java:159)
	at clojure.lang.AFn.applyTo(AFn.java:151)
	at clojure.core$apply.invoke(core.clj:601)
	at clojure.core$with_bindings_STAR_.doInvoke(core.clj:1771)
	at clojure.lang.RestFn.invoke(RestFn.java:425)
	at clojure.tools.nrepl.middleware.interruptible_eval$evaluate.invoke(interruptible_eval.clj:36)
	at clojure.tools.nrepl.middleware.interruptible_eval$interruptible_eval$fn__374$fn__376.invoke(interruptible_eval.clj:162)
	at clojure.core$comp$fn__4034.invoke(core.clj:2278)
	at clojure.tools.nrepl.middleware.interruptible_eval$run_next$fn__367.invoke(interruptible_eval.clj:129)
	at clojure.lang.AFn.run(AFn.java:24)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1110)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:603)
	at java.lang.Thread.run(Thread.java:636)


 Comments   
Comment by Joe Snikeris [ 22/Jul/12 11:23 AM ]

I'm not sure which would be better: behave as if one wasn't passed in or return an error on the transport.

Comment by Chas Emerick [ 20/Aug/12 9:40 PM ]

Fixed to send an error with a status of "namespace-not-found".





[NREPL-22] CLONE - set stack size for Android Created: 30/Jun/12  Updated: 24/Jul/12  Resolved: 24/Jul/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.0.5
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Hank Assignee: Chas Emerick
Resolution: Duplicate Votes: 0
Labels: Android
Environment:

Android



 Description   

If you want to use nREPL on Android (currently only possible with sattvik's fork of Clojure) then you need bigger stack sizes than Android's default 8k to eval even moderately complex expressions. For comparison: A regular JVM on a PC has default stack sizes around 256k ... 512k depending on platform.

In nrepl.clj, modify

(doto (Thread. r)
(.setDaemon true))))))

to read

(doto (Thread. (.getThreadGroup (Thread/currentThread)) r "nREPL" 524288) ; 512k generous stack size
(.setDaemon true))))))

Note: There are warnings here http://bit.ly/u86tF1 about choosing a good stack size but since in practice nREPL seems to have <10 threads running with 1 active client connection we can be generous here.

The more advanced version would be to make this somehow configurable but that'd probably be over-engineering things.



 Comments   
Comment by Hank [ 30/Jun/12 9:21 AM ]

I understand that after a brief detour via agents we're now back to creating our own threads in nREPL. So the above applies gain. Apparently in JIRA you can't reopen issues so I'm 'cloning' this one.

Comment by Chas Emerick [ 24/Jul/12 4:52 AM ]

I was able to reopen NREPL-8; let's track it there.





[NREPL-21] Sometimes *err*/*out*/value messages are surprisingly ordered Created: 17/Jun/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Flush-out-err-before-sending-value.patch    
Patch: Code

 Description   

As described here: https://github.com/technomancy/leiningen/issues/631, the *out*, *err*, and value printing are ordered differently than expected sometimes.

There are 2 things left here:
(1) The return value can sometimes print before synchronous prints to err/out. Synchronizing all the printing on the REPLy side doesn't seem to help. An example case:

=> (do (.print *err* "one") (print "two") (print "three"))
niltwothreeone

(2) Calling .println() on the *out*/*err* streams doesn't necessarily flush the stream. Changing this may have performance/network ramifications, but generally it seems like surprising behavior. An example case:

user=> (do (.println *out* "one") (.print *err* "two") (.print *err* "three") 1)
twothreeone
1

I'd expect two .print() calls to different streams to have undefined ordering, but the other orderings were all surprising.

The attached patch forces flushes to avoid both of these, for (1) immediately before sending the value message, and for (2) on the "normal" PrintStream [autoFlush] cases of println, printf, or format.

Side note: I couldn't get any tests to fail in this scenario, even after doing some reworking of the streams used in tests, but I kept one new test in this patch anyway. Turns out it's hard to write tests that really make sure .flush() gets called.



 Comments   
Comment by Chas Emerick [ 20/Jun/12 6:58 AM ]

Merged, thanks!





[NREPL-20] Stray "#" in session-out is breaking (.println *out*) Created: 09/Jun/12  Updated: 09/Jun/12  Resolved: 09/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alex Osborne Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Patch: Code and Test
Waiting On: Chas Emerick

 Description   

There's a stray "#" in clojure.tools.nrepl.middleware.session/session-out which is turning the body of the (not off) case into a lambda that's never executed:

(cond
(number? x) (.append buf (char x))
(not off) #(.append buf x) ; <----
(instance? CharSequence x) (.append buf ^CharSequence x off len)
:else (.append buf ^chars x off len))

https://github.com/clojure/tools.nrepl/blob/master/src/main/clojure/clojure/tools/nrepl/middleware/session.clj#L34

This is causing the newline character in (.println out) and (.println err) to be lost:

user=> (with-open [conn (repl/connect :port 7888)]
(-> (repl/client conn 1000)
(repl/message {:op :eval :code "(do (.println out \"1\") (.println out \"2\") (.println out \"3\")(flush))"})
doall
pprint))
({:out "123", ;; <--- where are my "\n"s?
...}, ...)

Removing the stray "#" fixes the problem:

{:out "1\n2\n3\n",
...}

Reported downstream by jaceklaskowski in https://github.com/technomancy/leiningen/issues/631

Patch with the trivial fix and unit test here: https://wok.meshy.org/~ato/0001-Remove-stray-hash-in-session-out-that-breaks-println-out.patch

Couldn't attach the patch to this issue as JIRA told me I don't have permission. I (Alex Osborne) have signed the Clojure CA and contribute this patch as per the terms of it.



 Comments   
Comment by Colin Jones [ 09/Jun/12 9:44 AM ]

Looks good, nice catch. Verified this test fails before the fix.

Chas, want me to apply/push this? I'm happy to do that, but wanted to wait since we haven't really talked about project workflow.

Comment by Chas Emerick [ 09/Jun/12 10:54 AM ]

Thank you very much — patch applied to master. I'll cut a new beta release on Monday so downstreams can pick it up.

Good eye, by the way, I'm sure I could have looked for days for that.





[NREPL-19] Android: nREPL starts with no namespace Created: 11/May/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Yakushev Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: android,, bug
Environment:

Android w/ Clojure 1.4.0, REPLy client / Eclipse CCW client



 Description   

When I start the nREPL on the Android device (by calling `(clojure.tools.nrepl.server/start-server :port 9999)`) it all goes well. But when I try to connect to this REPL using any of the clients I find myself in an empty namespace (or something like that). The var ns is unbound, no functions from the clojure.core are available. At the beginning REPLy tries to perform some actions but they fail (says that it cannot find symbol `defn` - because nothing from clojure.core is being mapped).

However I can do (in-ns 'anywhere) it works. Everything else in the REPL works correctly (as far as I see). The issue itself is minor but I'm afraid that it is caused by some crash during nREPL initialization that might lead to other problems in future.



 Comments   
Comment by Alexander Yakushev [ 11/May/12 11:06 AM ]

Can't edit, I meant the *ns* var, of course.

Comment by Alexander Yakushev [ 19/May/12 4:32 PM ]

With the help of Daniel Solano Gómez I managed to fix this bug. The problem was caused by the lack of user namespace in the Android-patched Clojure. nREPL assumes that the user namespace is present and uses it by default.

Here's the so called fix I ended up with:

...
(let [user-ns (create-ns 'user)]
  (binding [*ns* user-ns]
    (clojure.tools.nrepl.server/start-server :port 9999)))
...

The issue can be closed now.

Comment by Chas Emerick [ 27/May/12 4:04 PM ]

A follow up Q: user is created by clojure.lang.RT's static initialization. Is the lack of that in "Android-patched Clojure" an optimization of some sort?

Comment by Alexander Yakushev [ 27/May/12 4:49 PM ]

Exactly, Daniel Solano Gómez removed it because it's initialization took additional time, it seems.





[NREPL-18] Flush *out*/*err* after N characters Created: 18/Apr/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Flush-output-after-session-out-limit-characters.patch    
Patch: Code and Test

 Description   

Not sure how this should be configured, but it would be nice if doing something like (println large-seq-here) would flush output automatically after some number of characters. This is so that output could stream constantly across the nrepl connection instead of waiting for the entire output to send as one message (assuming no manual flushes).



 Comments   
Comment by Colin Jones [ 28/Apr/12 1:31 PM ]

There is a precedent for this, in the sense that many output buffers have some fixed size, and they are flushed when that size has been reached (e.g. http://www.gnu.org/software/libc/manual/html_node/Flushing-Buffers.html, http://docs.oracle.com/javase/6/docs/api/java/io/BufferedWriter.html)

We had talked about potential worries / headaches, where users may get partial lines printed at a time. But after more thought about it, I'm thinking we may be OK: concurrent out/err writes can already interleave in interesting ways, and depending on the placement of (flush) calls (explicitly or implicitly) to make them more atomic is something I'd already have expected to be fragile. Maybe others will see things I don't, though.

The implementation I have uses 1024 as the default, but that's pretty arbitrary. A less dynamic way of setting the limit would be also be fine with me, though since we're already in I/O-land, I'd assume performance isn't going to be bound by the dynamic var lookup.

Comment by Colin Jones [ 17/Jun/12 6:09 PM ]

Any thoughts on this one? It would be great to be able to print big seqs incrementally.

Comment by Chas Emerick [ 20/Jun/12 7:39 AM ]

Sorry for the delay, this fell through the cracks.

Looks good to me, except: can we make the "buffer size" an argument to the session middleware fn? That's where people will want to be able to customize it, if ever.

Commit and close this at will.

Comment by Colin Jones [ 20/Jun/12 11:14 PM ]

Done, thanks a bunch!





[NREPL-17] Agent nesting causes issues Created: 13/Apr/12  Updated: 16/Apr/12  Resolved: 16/Apr/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Blocker
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

We talked about this briefly on IRC/github: https://github.com/overtone/overtone/issues/85. I guess the nicest way to fix it would be to stop using agents in nREPL. There's only one spot where agents are created, and a few send-off calls (no sends), all internal, so any solution will be non-intrusive from an API perspective.

One implementation idea is to use a simple blocking queue and thread pool in java-land.

Another is to use nearly the same implementation as Agent, but remove the nesting restriction (call it NestableAgent?). Also get rid of the STM interaction and validator business since those aren't available outside the clojure.lang package - and then provide a parallel version of send-off. I've spiked a bit on this one, but I'm starting to think a plain queue/thread pool is cleaner.

Thoughts?



 Comments   
Comment by Chas Emerick [ 13/Apr/12 5:18 AM ]

Yes, I started working on this yesterday. Agents have definitely proven themselves unsuitable here, and a poor choice.

I'm not going to be fancy; we don't need fancy here.

Comment by Colin Jones [ 13/Apr/12 5:49 AM ]

Great news! Thanks for the quick response.

Comment by Chas Emerick [ 16/Apr/12 3:08 PM ]

Fixed in 0.2.0-beta6.





[NREPL-16] nrepl.middleware.interruptible_eval/interruptible_eval raises a stack inconsistence Exception if the call to clojure.main/repl fails Created: 12/Apr/12  Updated: 20/Apr/12  Resolved: 16/Apr/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Alexander Yakushev Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: bug
Environment:

clojure 1.4.0-beta5, Android



 Description   

If the call to clojure.main/repl in `evaluate' raises an exception before the :init part gets evaluated (so the expression `(push-thread-bindings @bindings)' is executed) then nREPL crashes with the following exception:

java.lang.IllegalStateException: Pop without matching push

It happens because after the underlining exception in clojure.main/repl is caught by the try block in `evaluate', the `finally' black calls (pop-thread-bindings) which were not actually "pushed".



 Comments   
Comment by Chas Emerick [ 16/Apr/12 5:34 PM ]

Fixed in 0.2.0-beta6. Please give it a try and see how it works on Android.

Comment by Alexander Yakushev [ 20/Apr/12 1:40 PM ]

It is OK now. OK in a sense that if something wrong happens inside the clojure.main/repl function then the stacktrace points there after the application dies.
Thanks for you help!





[NREPL-15] Allow clients to specify an ID for newly-retained sessions Created: 29/Mar/12  Updated: 12/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

...particularly important where less sophisticated clients and polling-oriented transports are involved (e.g. HTTP).






[NREPL-14] Exception when running -main from clojure.tools.nrepl.cmdline Created: 06/Mar/12  Updated: 20/Jun/12  Resolved: 20/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.2.0
Fix Version/s: 0.2.0

Type: Defect Priority: Major
Reporter: Trent Ogren Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Fix-calls-using-old-function-signatures.patch    
Patch: Code

 Description   

Running -main from clojure.tools.nrepl.cmdline gives the following exception:

Exception in thread "main" java.lang.IllegalArgumentException: No value supplied for key: 0
at clojure.lang.PersistentHashMap.createWithCheck(PersistentHashMap.java:89)
at clojure.core$hash_map.doInvoke(core.clj:355)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:600)
at clojure.tools.nrepl.server$start_server.doInvoke(server.clj:65)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.tools.nrepl.cmdline$_main.doInvoke(cmdline.clj:74)
at clojure.lang.RestFn.invoke(RestFn.java:397)
at clojure.lang.Var.invoke(Var.java:397)
at clojure.lang.AFn.applyToHelper(AFn.java:159)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.core$apply.invoke(core.clj:600)
at clojure.main$main_opt.invoke(main.clj:323)
at clojure.main$main.doInvoke(main.clj:426)
at clojure.lang.RestFn.invoke(RestFn.java:421)
at clojure.lang.Var.invoke(Var.java:405)
at clojure.lang.AFn.applyToHelper(AFn.java:163)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.main.main(main.java:37)

It looks like the repl/connect and start-server functions have changed since cmdline.clj was written. I attached a patch that updates their usage.



 Comments   
Comment by Chas Emerick [ 20/Jun/12 7:36 AM ]

Merged, thanks!





[NREPL-13] nrepl should support binding to a specific interface instead of all Created: 29/Feb/12  Updated: 29/Mar/12  Resolved: 29/Mar/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Toby Crawley Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Patch: Code

 Description   

nrepl currently accepts a :port to bind to as an option, and uses that with the single-arg ServerSocket constructor, which binds to that port on all interfaces. I propose we add a :host option as well that can be used to specify the hostname or address of the interface to bind to.

Here is a potential implementation:

diff --git a/src/main/clojure/clojure/tools/nrepl/server.clj b/src/main/clojure/clojure/tools/nrepl/server.clj
index 9dbac40..b6f625b 100644
--- a/src/main/clojure/clojure/tools/nrepl/server.clj
+++ b/src/main/clojure/clojure/tools/nrepl/server.clj
@@ -8,7 +8,7 @@
                                             pr-values
                                             session))
   (:use [clojure.tools.nrepl.misc :only (returning response-for log)])
-  (:import (java.net Socket ServerSocket)))
+  (:import (java.net InetAddress Socket ServerSocket)))
 
 (defn unknown-op
   "Sends an :unknown-op :error for the given message."
@@ -64,7 +64,9 @@
 
 (defn start-server
   "Starts a socket-based nREPL server.  Configuration options include:
- 
+
+   * :host — the address or hostname of the interface that should be used;
+       defaults to all interfaces
    * :port — defaults to 0, which autoselects an open port on localhost
    * :handler — the nREPL message handler to use for each incoming connection;
        defaults to the result of (default-handler)
@@ -77,8 +79,10 @@
 
    Returns a handle to the server that is started, which may be stopped
    either via `stop-server`, (.close server), or automatically via `with-open`."
-  [& {:keys [port transport-fn handler ack-port greeting-fn] :or {port 0}}]
-  (let [ss (ServerSocket. port)
+  [& {:keys [host port transport-fn handler ack-port greeting-fn] :or {port 0}}]
+  (let [ss  (if host
+              (ServerSocket. port 0 (InetAddress/getByName host))
+              (ServerSocket. port))
         smap {:ss ss
               :transport (or transport-fn t/bencode)
               :greeting greeting-fn

I'm putting a CA in the mail today.



 Comments   
Comment by Chas Emerick [ 29/Mar/12 12:53 PM ]

Fixed on master, released as part of 0.2.0-beta3.





[NREPL-12] Intermittent test failures for stdin Created: 26/Feb/12  Updated: 10/Jun/12  Resolved: 10/Jun/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Critical
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Improve-timing-issues-in-tests.patch    

 Description   

Failures like these are happening sometimes in the test suite, mainly around stdin:

FAIL in (request-multiple-read-objects-in) (nrepl_test.clj:211)
expected: (= (quote (:ohai)) (response-values (for [resp (repl-eval session "(read)")] (do (when (-> resp :status set (contains? "need-input")) (session {:op :stdin, :stdin ":ohai :kthxbai\n"})) resp))))
actual: (not (= (:ohai) nil))

FAIL in (request-multiple-read-objects-in) (nrepl_test.clj:217)
expected: (= [" :kthxbai"] (repl-values session "(read-line)"))
actual: (not (= [" :kthxbai"] nil))

I've done a ton of debugging and thinking about this, and haven't gotten far. A few theories I had that seemed decent at the time, but ultimately haven't gotten rid of these occasional failures:

The debugging I've been doing (print statements galore) suggests that when this problem happens, the client does send back the proper :stdin, but it doesn't get picked up by the add-stdin handler in time to write to the :stdin-writer and avoid the expiring timeout.

I'm pretty much stumped at this point, unfortunately.



 Comments   
Comment by Chas Emerick [ 29/Mar/12 4:18 PM ]

This is becoming a serious problem; the build fails half the time on build.clojure.org, making it incredibly frustrating to perform releases, etc.

Any new ideas / evaluation of this is most welcome.

Comment by Colin Jones [ 15/Apr/12 10:24 PM ]

My current vague suspicion is that it's a CPU contention thing between the server and client.

It's kind of hard for me to reason about the execution of the response-values lazy seq, and where the blocking actually happens. And of course adding logging changes the behavior sometimes. I wouldn't be opposed to adding sleeps in there, but I expect those kinds of things would still be flaky later, on build machines in particular :/

Incidentally, bumping the client timeout up to Long.MAX_VALUE helps a bit the stdin failures, but doesn't solve them. It does create new ones: session-lifecycle, unknown-op, and read-timeout block forever trying to reduce across the whole seq in combine-responses. It was a bit surprising to me to see that reduce in the response-values pipeline, and I pursued that at one point as well, but it hasn't gotten me anywhere. Decreasing the timeout helps to reproduce the failures.

I got this thread dump from a blocking-forever stdin test (run via `lein test clojure.tools.nrepl-test`): https://refheap.com/paste/2141.

Comment by Colin Jones [ 03/Jun/12 5:07 AM ]

With this patch, I was able to run clojure.tools.nrepl-test 100 times successfully. Previous attempts would generally only get as far as 15 or so before failing for one reason or another.

It also gets rid of a race condition with my skip-if-eol stuff that was addressing multiple input reads. Lastly, it avoids the need for the 10-second timeout, so the tests are faster now as well.

Comment by Chas Emerick [ 09/Jun/12 10:57 AM ]

I'm psyched that you may have found the solution to these problems. I've applied the patch locally, and it tests well. I want to eyeball it for a minute (probably tomorrow) before moving on.

Comment by Chas Emerick [ 10/Jun/12 4:40 PM ]

Pushed, will be in the next beta release. Thanks so much!





[NREPL-11] Ensure efficient bytestring (byte[]) support in bencode transport Created: 21/Feb/12  Updated: 21/Aug/12  Resolved: 21/Aug/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

In particular, no boxing of individual bytes or multiple en/decoding steps should be involved.

This implies that:

1. all string values should remain byte[] coming out of the bencode protocol support (which makes it more compliant with bencode anyway)
2. the string decoding should occur in the bencode transport impl
3. all string values should be decoded, except for those named in a reserved slot (say, "-unencoded")

All map keys should always be decoded to Strings.

While nREPL should continue to be fully compatible with Clojure 1.2.x, let's not kill ourselves trying to optimize for 1.2.x as well as Clojure >= 1.3.0; if necessary, just make sure the latter is fast. If 1.2.x benefits as well, then good.



 Comments   
Comment by Meikel Brandmeyer [ 22/Feb/12 2:05 PM ]

How about “nrepl/unencoded” re point 3?

Comment by Chas Emerick [ 22/Feb/12 5:57 PM ]

Is that intended to become a namespaced keyword at some point (further up in the stack than the bencode parser or transport, of course)?

If so, does that imply that other keys (perhaps "nonstandard" ones defined by middlewares) should be namespaced as well?

Also, this is actually impacting the protocol; nrepl/* makes me think the slot is related to an "application-level" handler provided by nREPL.

FWIW, my suggestion of -unencoded was a nod to Python's _foo and Clojure's -foo to denote private / internal members. *shrug*

Comment by Meikel Brandmeyer [ 23/Feb/12 5:03 AM ]

Namespaced keywords are indeed the inspiration. So that it should probably be more like clojure.tools.nrepl.transport/unencoded rather than just nrepl/unencoded. However it is not a requirement to be a real keyword. A qualified string works just as well. There could be a middleware which turns the map keys into keywords (considering also a possible namespace). ring has a similar middleware, IIRC. I think that qualified operation and result field values (in some form or the other) are a best practise in this respect.

However your argument of unencoded being part of the protocol is valid. But then why should it be private? Document it in the official protocol spec and let middlewares also add fields to it. The question is then: what happens with nested structures (eg. :status, which has to be traversed now painfully)? If middlewares weren't allowed to add to it, then only :value would be an interesting target for unencoded. So a simple flag would suffice.

In general, I still think a tagged protocol would have been the better solution since it also allows nesting and each slot knows whether it's encoded or not.

Maybe we should clarify what middlewares are allowed to do. Is there already some documentation on this? (I'm must confess I'm not up-to-date with all the design changes.)

Comment by Chas Emerick [ 01/Mar/12 3:32 PM ]

Re: encoding, I've started to think perhaps nothing should be decoded by default, and middlewares can independently handle such things. The standard middlewares will stake out one approach, but that at least leaves open the possibility of a different convention emerging over time that can be adopted later.

Opinions?

Comment by Chas Emerick [ 20/Aug/12 4:14 PM ]

Sorry, where is the code for this? No .patch here, and nothing in bitbucket looks right.

Leaving aside the code for now, defaulting to unencoded strings continues to seem reasonable; a default Transport impl/substrate can perform the en/decoding, as well as establish a default convention for indicating that some fields values should be left unencoded and should not be decoded.

Comment by Meikel Brandmeyer [ 20/Aug/12 4:21 PM ]

The code is here: https://github.com/kotarak/tools.nrepl/tree/bencode-improvements2

Comment by Chas Emerick [ 20/Aug/12 10:14 PM ]

Looks good, patch applied.

I'm less and less sure re: -unencoded. Thinking back to your "tagged protocol" comment, adopting a convention of keys like "value/binary" (or something similar) would be quite a bit more flexible and somewhat simpler to handle.

(Thinking of rich content, mime types could then be sent in e.g. "value/mime" — not that the transport would care about that, but it would at least be easy to pair up data with its "tag".)

Thoughts?

Comment by Chas Emerick [ 21/Aug/12 8:26 PM ]

Releasing -beta9 now; further discussion of unencoded value semantics can continue elsewhere…





[NREPL-10] SocketExceptions on transport/bencode when the other end has gone away Created: 19/Feb/12  Updated: 12/Oct/12  Resolved: 12/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0, 0.2.0-RC1

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

When running the tests, I'm occasionally getting SocketExceptions (which don't fail the test suite) with the message "Broken pipe". I think these stem from one end of the nREPL socket trying to write to the socket after the other end has gone away.

There's an existing test (ensure-closeable) that explicitly expects a SocketException to be raised if the socket has had .close called directly on it, rather than having the other end hang up, which I think is what the "Broken pipe" tells us. I'm curious as to whether this is important for some reason I haven't seen, or whether catching all SocketExceptions on transport/bencode's write function and returning nil would be appropriate.

If the end doing the socket closing actually should affect the behavior, we could sniff the exception message, but that feels a bit yucky.

I can actually reproduce this in "real life" by firing up an nREPL server / client (`reply --nrepl --port 9999`), then attaching to it (`reply --nrepl --attach 9999`), and in the attached client, run (Thread/sleep 10000) and immediately kill the process. Then the "Broken pipe" exception shows up in the original REPL with the server in the same process.

I've got a patch for this in my bitbucket fork (socket-exception branch): https://bitbucket.org/trptcolin/nrepl/changeset/4a432f3ce95a



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:35 AM ]

What do we really want to happen when a SocketException (or really, any error) occurs?

I'm all for better error handling/reporting, but it shouldn't obfuscate what's going wrong. Catching and squelching isn't doing anyone any favors. That could be particularly confusing if done at the transport level. "You mean all the messages I've been sending have been dropped on the floor silently, and I was supposed to be checking for a nil return?"

Comment by Colin Jones [ 26/Feb/12 12:46 PM ]

Yeah, you're right. Although, isn't checking for a nil return already necessary to account for timeouts?

What about having a server-side transport error handler that's rebindable, and re-throws by default? Or maybe we could collect errors somewhere, similar to what clojure.core/agent-error does.

The more I think about it, the less important this seems, though - mainly an annoyance when running tests.

Comment by Chas Emerick [ 12/Oct/12 12:51 AM ]

Was simply a disconnection stacktrace due to a client closing up prior to an evaluation response being written to the transport.

Fixed in https://github.com/clojure/tools.nrepl/commit/2d90bdd4fe14298bbfcaab52ecdea48781c71604





[NREPL-9] (read-line) doesn't prompt for input after a (read) Created: 19/Feb/12  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Colin Jones Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

When (read-line) is preceded by (read), it already has a newline in the stream, and returns "" immediately rather than prompting for input.

(read) then (read-line) works fine in the stock Clojure repl because the REPL shares STDIN with the in used by the normal (read) and (read-line) calls. So when the REPL's loop does clojure.main/repl-read (to get my evaluation input "(read-line)"), it skips whitespace before the read for the thing to eval, reads that thing, and then skips a newline, if that's the next thing: https://github.com/clojure/clojure/blob/master/src/clj/clojure/main.clj#L145-161

That allows the .read calls generated by the (read-line) execution to start after a trailing newline left in the stream.

We can do the same thing in nrepl.middleware.session/add-stdin on an eval message, but we need to check .ready first, to avoid sending unnecessary :need-input requests back to the client. I think this does belong in add-stdin, since it's essentially a stdin concern, even though it acts on the eval message.

I've got one test exposing the problem, and one proving we only eat newlines, not any old trailing character. I had to stick :stdin-reader on the session to enable consuming the newline from the middleware - not sure whether there's a way to do without it that I'm not seeing.



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:17 AM ]

Thanks for chasing this down! Patch applied to master.





[NREPL-8] set stack size for Android Created: 30/Dec/11  Updated: 03/Oct/12  Resolved: 03/Oct/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: 0.0.5
Fix Version/s: 0.2.0-beta9

Type: Enhancement Priority: Minor
Reporter: Hank Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: Android
Environment:

Android



 Description   

If you want to use nREPL on Android (currently only possible with sattvik's fork of Clojure) then you need bigger stack sizes than Android's default 8k to eval even moderately complex expressions. For comparison: A regular JVM on a PC has default stack sizes around 256k ... 512k depending on platform.

In nrepl.clj, modify

(doto (Thread. r)
(.setDaemon true))))))

to read

(doto (Thread. (.getThreadGroup (Thread/currentThread)) r "nREPL" 524288) ; 512k generous stack size
(.setDaemon true))))))

Note: There are warnings here http://bit.ly/u86tF1 about choosing a good stack size but since in practice nREPL seems to have <10 threads running with 1 active client connection we can be generous here.

The more advanced version would be to make this somehow configurable but that'd probably be over-engineering things.



 Comments   
Comment by Chas Emerick [ 30/Dec/11 9:14 AM ]

Noted, thank you. I'll try to make sure a provision for this gets into the next significant release.

My first inclination is to ensure that the stack size is configurable; a hardcoded size would likely be just as inappropriate in some environments as the system default.

Comment by Chas Emerick [ 21/Feb/12 5:21 AM ]

nREPL is now using Clojure's built-in agent threadpools. This should theoretically simplify things for you, correct? i.e. Isn't a patched build of Clojure necessary for use on Android for this stack size issue, among other reasons?

Comment by Hank [ 22/Feb/12 7:21 AM ]

The patched build is only necessary for the JVM byte code that the Clojure compiler dynamically generates to be converted to Dalvik bytecode. However adding additional android patches such as configuring the agent threadpool with a larger stack size do make sense. I'm not the official Android Clojure maintainer though – I guess there isn't an official one really – so my opinion on this only goes so far.

Comment by Chas Emerick [ 22/Feb/12 8:11 AM ]

Well, I think you have a solid case for expanding the scope of the android fork, or even for a change to parameterize the threadpools in Clojure itself.

I'm going to close this; there's really nothing to be done from within nREPL given that it doesn't maintain its own threadpool anymore.

Comment by Chas Emerick [ 24/Jul/12 4:49 AM ]

Reopening; since the use of agents was a fool's errand, parameterizing stack size needs to happen.

Comment by Chas Emerick [ 15/Aug/12 3:58 PM ]

Now that I look at this again, there are a number of options in nREPL to solve this.

First, you can provide an :executor argument to the interruptible-eval middleware. This could be any java.util.concurrent.Executor, so you can configure it to use whatever threads you want.

Second, and if you want to minimize work and retain as much of nREPL's default configuration otherwise, you can patch in your own thread pool by binding #'clojure.tools.nrepl.middleware.interruptible-eval/configure-thread-factory like so:

(def thread-group ...)
(def stack-size ...)

(defn android-thread-factory
  "Returns a new ThreadFactory suitable for use with android. This implementation
   generates daemon threads, with names that include the session id."
  []
  (let [session-thread-counter (AtomicLong. 0)]
    (reify ThreadFactory
      (newThread [_ runnable]
        (doto (Thread. thread-group
                runnable
                (format "nREPL-worker-%s" (.getAndIncrement session-thread-counter))
                stack-size)
          (.setDaemon true))))))

(with-bindings {#'clojure.tools.nrepl.middleware.interruptible-eval/configure-thread-factory
                android-thread-factory}
  (clojure.tools.nrepl.server/start-server ...))

The above totally untested, but I hope you get the idea. Give it a shot, and let me know how it goes. I'm going to take off this issue's fix target for now.

Comment by Chas Emerick [ 03/Oct/12 7:03 AM ]

I hear good things at this point about people connecting to nREPL endpoints running on Android. Please file another issue if this or other Android-specific problems arise in the future.





[NREPL-7] Dependency on 1.3.0-master-SNAPSHOT instead of 1.3.0 Created: 15/Nov/11  Updated: 15/Nov/11  Resolved: 15/Nov/11

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Steve Lindsay Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Running "mvn install" on tools.nrepl results in a build failure:

[INFO] Missing:
[INFO] ----------
[INFO] 1) org.clojure:clojure:jar:1.3.0-master-SNAPSHOT

Changing 1.3.0-master-SNAPSHOT to 1.3.0 in src/integration/clojure-1.3.0/pom.xml seems to fix it.



 Comments   
Comment by Chas Emerick [ 15/Nov/11 5:13 AM ]

Bizarre, since it is building cleanly here and on build.clojure.org.

I should certainly update the version number in that pom to 1.3.0 though. I'll do that shortly.

FWIW, maybe adding a -U to your mvn invocation would help? That forces a check of all snapshots.

Comment by Chas Emerick [ 15/Nov/11 5:22 AM ]

fixed on HEAD





[NREPL-6] Add simple HOWTO to README for both tooling and application developers Created: 30/Sep/11  Updated: 21/Nov/12  Resolved: 21/Nov/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

...maybe push the protocol details into a /doc directory.






[NREPL-5] Add support for reading from stdin Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

Swank does this by polling back to the connected client to request input. In the face of asynchronous operations, this may be the only option.

Will require adding to the protocol.



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:58 AM ]

See clojure.tools.nrepl.middleware.session/add-stdin for the implementation. Userland documentation coming.





[NREPL-4] Provide sane multiplexing of output in the face of multithreaded, asynchronous operation Created: 30/Sep/11  Updated: 03/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Basically, (send-off some-agent println) & co. should get what's printed to out in that agent's thread back to the nREPL client — not silently let it dump out to System/out.

Portal ostensibly does this well. Examine their approach, see if it is compatible with nREPL's objectives.

Ill-formed brain dump:

  • multiplex new out's to System/out
    • (still won't solve clojure.test/test-out content will disappearing into the ether when it's loaded when out is bound to an nREPL out; maybe we should ensure out is bound to System/out while code is being loaded?)
  • optionally multiplex System/out and System/err
  • optionally join multiplexed S/out and S/err, receive :stdout, :stderr msgs





[NREPL-3] Adopt default port Created: 30/Sep/11  Updated: 22/Oct/12

Status: Open
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Unresolved Votes: 0
Labels: None


 Description   

7888 is "free", at least in IANA.

Most users want to put an nREPL port on their app, and that should always be on a typical port. Auto-selection of a port is only desirable in tooling scenarios, and tooling authors can pass an argument to start-server.

This will change the behaviour of (start-server).






[NREPL-2] send-ack probably leaks connections/sockets Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Critical
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

The let in send-ack should be be with-open.






[NREPL-1] Whitespace is not printed via :out Created: 30/Sep/11  Updated: 21/Feb/12  Resolved: 21/Feb/12

Status: Closed
Project: tools.nrepl
Component/s: None
Affects Version/s: None
Fix Version/s: 0.2.0

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Chas Emerick
Resolution: Completed Votes: 0
Labels: None


 Description   

This:

=> (println " ")
nil

Should be this:

=> (println " ")
 
nil

Perhaps :out isn't being sent if the content {{.trim}}s down to a zero-length string?



 Comments   
Comment by Chas Emerick [ 21/Feb/12 5:54 AM ]

Resolved in the redesign.





[MTOWER-3] BigDecimal: unnecessary reflection used in math functions for coercion (bigint) Created: 13/May/13  Updated: 13/May/13

Status: Open
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jürgen Hötzel Assignee: Mark Engelberg
Resolution: Unresolved Votes: 0
Labels: patch,

Attachments: Text File 0001-Use-coercion-and-rounding-methods-provided-by-BigDec.patch    
Patch: Code

 Description   

This improves performance, because no reflection has to be used (bigint):

Patched:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 1640.177041 msecs"
Orig:
clojure.math.numeric-tower> (time (let [b (BigDecimal. "2.2")] (dotimes [x 10000000] (round b))))
"Elapsed time: 3498.339271 msecs"






[MTOWER-2] Eliminate a few uses of Reflection in math.numeric-tower Created: 28/Oct/12  Updated: 23/Nov/12  Resolved: 23/Nov/12

Status: Resolved
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Andy Fingerhut Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File mtower-2-eliminate-reflection-v1.txt    

 Description   

There are a few cases where calls to .numerator and .denominator on clojure.lang.Ratio's cause reflection, because the Ratio is not type hinted as such.



 Comments   
Comment by Andy Fingerhut [ 28/Oct/12 7:48 PM ]

mtower-2-eliminate-reflection-v1.txt dated Oct 28 2012 eliminates a few instances of reflection in math.numeric-tower.

Comment by Mark Engelberg [ 23/Nov/12 3:30 AM ]

Applied patch, updated version number to 0.0.2





[MTOWER-1] README should be updated to conform to contrib standard Created: 16/Sep/12  Updated: 17/Sep/12  Resolved: 17/Sep/12

Status: Resolved
Project: math.numeric-tower
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Christian Romney Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: documentation, patch

Attachments: Text File 0001-Improved-README-to-follow-standard-for-contrib.patch     Text File 0001-Improved-README-to-follow-standard-for-contrib.patch    
Patch: Code

 Description   

As per Sean Corfield's suggestion here: https://groups.google.com/forum/?fromgroups=#!searchin/clojure-dev/math/clojure-dev/p5oz42gR_sk/cesMHO9cDWEJ
the math.numeric-tower README.md should be updated to be more useful, especially to newbies.



 Comments   
Comment by Christian Romney [ 16/Sep/12 10:50 PM ]

Please ignore previous patch (doesn't format code examples correctly). This one looks better on Github. You can see it here: https://github.com/xmlblog/math.numeric-tower

Comment by Sean Corfield [ 17/Sep/12 11:33 AM ]

Patch applied.





[MCOMB-2] OutOfMemory Error with combinatorics/subsets Created: 29/Apr/13  Updated: 02/May/13

Status: Open
Project: math.combinatorics
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Mark Engel Assignee: Mark Engelberg
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac OS X 10.8.3
Clojure 1.5.1 (installed with homebrew)
Leiningen 2.1.3 on Java 1.6.0_45 Java HotSpot(TM) 64-Bit Server VM
-Xmx2048M
math.combinatorics 0.0.4

% java -version
java version "1.6.0_45"
Java(TM) SE Runtime Environment (build 1.6.0_45-b06-451-11M4406)
Java HotSpot(TM) 64-Bit Server VM (build 20.45-b01-451, mixed mode)


Attachments: Text File mcomb-2-allow-subsets-to-use-less-memory-v1.txt    

 Description   

Hello guys,

I have an issue with an OutOfMemory error with bigger sets and the subsets command.

I have bigger sets of 1000+ elements and want to lazily create all possible subsets of this set in order to filter out some that are interesting to me.

I was very happy to run into your library as it handles the heavy lifting for me and I only have to do the filtering. Nice!

But if I run this sample code

lein repl
=> (last (clojure.math.combinatorics/subsets (range 1000)))
OutOfMemoryError Java heap space  clojure.core/map (core.clj:2469)

it returns with an OutOfMemory Error to me.
I thought the memory usage of subsets would be constant as the function should return a lazy list.

It would be really nice if this library could calculate subsets of bigger lists without running into memory problems.

I posted this question on StackOverflow with more information:
http://stackoverflow.com/questions/16194841/clojure-lazy-sequences-in-math-combinatorics-results-in-outofmemory-oom-error

And people replied that they don't have this issue. Is this an issue with my platform or is this an issue with the memory usage of subsets?
It would be great if you could show me a way to get around this issue.

If I can be of any help, just let me know.

Cheers, Mark



 Comments   
Comment by Stefan du Fresne [ 29/Apr/13 5:29 AM ]

I get this as well, 2009 MBP, with any (range x) higher than 18.

Comment by Mark Engel [ 29/Apr/13 8:32 AM ]

Great to have the problem reproduced at another computer.

I just found an article describing an OutOfMemory Error with mapcat, which is used in the subsets function
http://clojurian.blogspot.de/2012/11/beware-of-mapcat.html.

Comment by Andy Fingerhut [ 02/May/13 2:30 PM ]

Patch mcomb-2-allow-subsets-to-use-less-memory-v1.txt dated May 2 2013 seems to fix this problem.

range returns a chunked sequence, and when map processes a chunked sequence it preserves the chunks. Chunks are little Java arrays of Object references, pointing at the results, and none of them will be GCed until the entire chunk is finished being processed.

It might be that a few other calls to unchunk wrapped around range calls might be useful in the math.combinatorics library, but certainly not all of them.





[MCOMB-1] math.combinatorics README should be updated to conform to contrib standard Created: 17/Sep/12  Updated: 17/Sep/12  Resolved: 17/Sep/12

Status: Resolved
Project: math.combinatorics
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Trivial
Reporter: Christian Romney Assignee: Mark Engelberg
Resolution: Completed Votes: 0
Labels: documentation, patch

Attachments: Text File 0001-Updated-README-to-conform-to-new-contrib-standard.patch    
Patch: Code

 Description   

As per Sean Corfield's suggestion here: https://groups.google.com/forum/?fromgroups=#!searchin/clojure-dev/math/clojure-dev/p5oz42gR_sk/cesMHO9cDWEJ
the math.combinatorics README.md should be updated to be more useful, especially to newbies.



 Comments   
Comment by Sean Corfield [ 17/Sep/12 11:35 AM ]

Patch applied.





[MATCH-69] AOT-compiling match expression produces stack overflow Created: 24/Apr/13  Updated: 24/Apr/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The offending match expression is pretty gnarly, will try to simplify and post it here later. For now, a (partial) stack trace, eliding the repeating bits:

Caused by: java.lang.StackOverflowError
	at clojure.core$map$fn__4207.invoke(core.clj:2479)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.ChunkedCons.chunkedNext(ChunkedCons.java:59)
	at clojure.lang.ChunkedCons.next(ChunkedCons.java:43)
	at clojure.lang.PersistentVector.create(PersistentVector.java:51)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:31)
	at clojure.core$vec.invoke(core.clj:354)
	at clojure.core.match.PatternMatrix.column(match.clj:713)
	at clojure.core.match$useful_p_QMARK_.invoke(match.clj:811)
	at clojure.core.match.PatternMatrix$iter__1458__1464$fn__1465$iter__1460__1466$fn__1467$fn__1468.invoke(match.clj:765)
	at clojure.core.match.PatternMatrix$iter__1458__1464$fn__1465$iter__1460__1466$fn__1467.invoke(match.clj:763)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core.match.PatternMatrix$iter__1458__1464$fn__1465.invoke(match.clj:764)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:67)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$partition$fn__4309.invoke(core.clj:2834)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$map$fn__4207.invoke(core.clj:2479)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.PersistentVector.create(PersistentVector.java:51)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:31)
	at clojure.core$vec.invoke(core.clj:354)
	at clojure.core.match.PatternMatrix.useful_matrix(match.clj:763)
	at clojure.core.match.PatternMatrix.necessary_column(match.clj:755)
	at clojure.core.match.PatternMatrix$choose_column__1441.invoke(match.clj:720)
	at clojure.core.match.PatternMatrix.compile(match.clj:738)
	at clojure.core.match$first_column_chosen_case$switch_clauses__1406$fn__1407.invoke(match.clj:643)
	at clojure.core$map$fn__4211.invoke(core.clj:2492)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core.protocols$seq_reduce.invoke(protocols.clj:30)
	at clojure.core.protocols$fn__6026.invoke(protocols.clj:54)
	at clojure.core.protocols$fn__5979$G__5974__5992.invoke(protocols.clj:13)
	at clojure.core$reduce.invoke(core.clj:6177)
	at clojure.core$into.invoke(core.clj:6229)
	at clojure.core.match$first_column_chosen_case$switch_clauses__1406.invoke(match.clj:646)
	at clojure.core.match$first_column_chosen_case.invoke(match.clj:676)
	at clojure.core.match.PatternMatrix.compile(match.clj:740)
	at clojure.core.match$first_column_chosen_case$switch_clauses__1406$fn__1407.invoke(match.clj:643)
	at clojure.core$map$fn__4211.invoke(core.clj:2492)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:484)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core.protocols$seq_reduce.invoke(protocols.clj:30)
	at clojure.core.protocols$fn__6026.invoke(protocols.clj:54)
	at clojure.core.protocols$fn__5979$G__5974__5992.invoke(protocols.clj:13)
	at clojure.core$reduce.invoke(core.clj:6177)
	at clojure.core$into.invoke(core.clj:6229)





[MATCH-68] Vector match "underflow" => IndexOutOfBoundsException Created: 22/Apr/13  Updated: 18/May/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File vector-rest-WIP.diff    
Patch: Code and Test

 Description   

This fails with 0.2.0-alpha12:

(match [[:x]]
  [[m n & _]] 1)

I expect nil, but an unguarded subvec call throws an IndexOutOfBoundsException.

A WIP patch is attached that fixes this (perhaps incorrectly) but which produced regressions (outside of core.match), e.g.

(match [[[:x "t"]]]
 [[[:x & a] & tail]] :a
 [[[:y & p] [:x & a] & tail]] :b)
=> nil

The patch also "fixes" this (by changing pattern-compare behaviour for rest VectorPattern s, but that leads to a regression in the vector-pattern-rest-2 testcase in core.match.

(Original discussion with further background here.)



 Comments   
Comment by Greg Chapman [ 24/Apr/13 5:26 PM ]

FWIW, I've been using the patch described here, and haven't yet run into any problems, though I also haven't used core.match that much. Anyway, with the change to subvec-inline, both of the cases here act as expected (producing nil and :a, respectively).

(I note looking at my patch that I'm doubly-evaluating ocr in the second overload; obviously that should be fixed with a let binding).

Comment by David Nolen [ 18/May/13 3:11 PM ]

The patch looks like it's going in the right direction. Can you explain why your patch without the pattern-compares case causes the second example to fail?





[MATCH-67] match broken on ClojureScript >= 88b36c1 Created: 14/Feb/13  Updated: 15/Feb/13  Resolved: 15/Feb/13

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

The ClojureScript example in core.match's readme is failing at the moment, using core.match 0.2.0-alpha11:

(ns foo.bar
  (:use-macros [clojure.core.match.js :only [match]]))

(match [(mod 5 3) (mod 5 5)]
      [0 0] "FizzBuzz"
      [0 _] "Fizz"
      [_ 0] "Buzz"
      :else n)

ClojureScript:foo.bar> WARNING: Use of undeclared Var foo.bar/e__4345__auto__ at line 4 
clojure.lang.ExceptionInfo: Unsupported binding form: (if (clojure.core/identical? e__4345__auto__ 0) (do (clojure.core/let [ocr-5230 (mod 5 5) ocr-5229 (mod 5 3)] (try (clojure.core/cond (clojure.core/= ocr-5230 0) (clojure.core/let [G__5226 ocr-5229] "Buzz") :else (throw 0)) (catch e__4345__auto__ (if (clojure.core/identical? e__4345__auto__ 0) (do (clojure.core/let [G__5228 ocr-5230 G__5227 ocr-5229] n)) (throw e__4345__auto__)))))) (throw e__4345__auto__)) at line 4  {:tag :cljs/analysis-error, :file nil, :line 4}

The same error is produced with any usage of match in ClojureScript AFAICT, not just the README example. However, all is well with core.match AFAICT under Clojure, and if you use older revs of ClojureScript, e.g. r1552. I did a bisect, and found that this is the first ClojureScript commit with which the example does not work:

https://github.com/clojure/clojurescript/commit/88b36c1

Reverting it with master results in proper evaluation of the match, modulo the warning.

I don't see any obvious reason why the above would cause any problems, and some moderate digging based on what I know about ClojureScript didn't yield any ah-ha! moments.



 Comments   
Comment by David Nolen [ 15/Feb/13 5:21 PM ]

fixed http://github.com/clojure/core.match/commit/3bab92b6620dccdcb9e55941af4599e3adf78a6e

Comment by Chas Emerick [ 15/Feb/13 7:51 PM ]

Thanks for looking at this!





[MATCH-66] Cannot match entire/single value Created: 06/Jan/13  Updated: 18/May/13  Resolved: 18/May/13

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Description   
=> (match 3 x x)
CompilerException java.lang.RuntimeException: Unable to resolve symbol: ocr-34984 in this context, compiling:(NO_SOURCE_PATH:1:1) 
=> (macroexpand '(match 3 x x))
(let* [x ocr-35001] x)

(Discussed briefly here.)



 Comments   
Comment by David Nolen [ 18/May/13 2:18 PM ]

fixed, http://github.com/clojure/core.match/commit/02a833efb959e0518f264ded3b98ce4215b5622c





[MATCH-65] cata matching Created: 23/Nov/12  Updated: 23/Nov/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Dan Friedman's pattern matcher has a nice feature called cata-matching - allowing recursive matching from the match itself. Useful when writing compilers.






[MATCH-64] Improve match compile times Created: 15/Aug/12  Updated: 15/Aug/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None





[MATCH-63] IllegalArgumentException when AOT compiling namespace using clojure.core.match since alpha10 Created: 08/Aug/12  Updated: 26/Feb/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Paudi Moriarty Assignee: David Nolen
Resolution: Unresolved Votes: 3
Labels: None
Environment:

core.match 0.2.0-alpha10 and 0.2.0-alpha11-SNAPSHOT (git 7ad66cc)
clojure 1.3.0 and 1.4.0


Attachments: Zip Archive matchtest.zip    

 Description   

Getting this when AOT compiling a trivial project (attached) using alpha10: (works fine with alpha9)

Caused by: java.lang.IllegalArgumentException: No method in multimethod 'to-source' for dispatch value: class clojure.core.match.WildcardPattern
	at clojure.lang.MultiFn.getFn(MultiFn.java:121)
	at clojure.lang.MultiFn.invoke(MultiFn.java:167)
	at clojure.core.match$dag_clause_to_clj.invoke(match.clj:424)
	at clojure.lang.AFn.applyToHelper(AFn.java:167)
	at clojure.lang.AFn.applyTo(AFn.java:151)
	at clojure.core$apply.invoke(core.clj:602)
	at clojure.lang.AFn.applyToHelper(AFn.java:167)
	at clojure.lang.RestFn.applyTo(RestFn.java:132)
	at clojure.core$apply.invoke(core.clj:604)
	at clojure.core$partial$fn__3796.doInvoke(core.clj:2343)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$map$fn__3811.invoke(core.clj:2430)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$apply.invoke(core.clj:600)
	at clojure.core$mapcat.doInvoke(core.clj:2459)
	at clojure.lang.RestFn.invoke(RestFn.java:423)
	at clojure.core.match.SwitchNode.n_to_clj(match.clj:446)
	at clojure.core.match.BindNode.n_to_clj(match.clj:411)
	at clojure.core.match$executable_form.invoke(match.clj:1713)
	at clojure.core.match$clj_form.invoke(match.clj:1721)
	at clojure.core.match$match.doInvoke(match.clj:1750)


 Comments   
Comment by Frederik De Bleser [ 20/Nov/12 4:18 PM ]

I can confirm the same issue on alpha11.

Comment by David Nolen [ 23/Nov/12 4:52 PM ]

I believe I may have found the cause and have fixed master - if someone can confirm I'll happily cut another release.

http://github.com/clojure/core.match/commit/cbcc6e5fa070a7025e72f5eab4e83eaec100332b

Comment by Paudi Moriarty [ 15/Jan/13 9:42 AM ]

Thanks David,

That change has not fixed the issue for me.

Comment by Paudi Moriarty [ 15/Jan/13 9:56 AM ]

So the pattern parameter being passed to dag-clause-to-clj:

(defn dag-clause-to-clj [occurrence pattern action]
  (let [test (if (instance? clojure.core.match.IPatternCompile pattern)
               (to-source* pattern occurrence)
               (to-source pattern occurrence))]
...

is a WildcardPattern which doesn't implement IPatternCompile and doesn't have a to-source method. So it seems it shouldn't be passed at all. Indeed, in normal usage it isn't.

;; ## Wildcard Pattern
;; 
;; A wildcard pattern accepts any value.
;;
;; In practice, the DAG compilation eliminates any wildcard patterns.

(defprotocol IWildcardPattern
  (sym [this]))

(deftype WildcardPattern [sym _meta]
  IWildcardPattern
  (sym [_] sym)
  clojure.lang.IObj
  (meta [_] _meta)
  (withMeta [_ new-meta]
    (WildcardPattern. sym new-meta))
  Object
  (toString [_]
    (str sym)))
Comment by David Nolen [ 15/Jan/13 11:17 AM ]

Can you include an example pattern which works under normal circumstances but fails under AOT? This must meant that WildcardPatterns are making it through under AOT and getting to the multimethod case but not under incremental compilation at the REPL.

Comment by Paudi Moriarty [ 16/Jan/13 9:34 AM ]

This seems to do it:

(match {}
  {:a :x :b b} :dummy
  :else :dummy)
Comment by Tim Olsen [ 26/Feb/13 1:55 PM ]

I've seen this problem as well. If I try to compile a second time, however, without running lein clean, the compilation succeeds. Maybe it's a bootstrapping problem?





[MATCH-62] ClojureScript map-matching should use cljs.core/ILookup, not cljs.core.ILookup Created: 30/Jun/12  Updated: 01/Jul/12  Resolved: 01/Jul/12

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs_lookup.patch    

 Description   

The latter form is an implementation detail and doesn't work in the latest cljs compiler release.



 Comments   
Comment by Kevin Lynagh [ 30/Jun/12 12:10 PM ]

Patch against master to use cljs.core/ILookup. Also available at https://github.com/lynaghk/core.match/tree/62-cljs-protocol

Comment by David Nolen [ 01/Jul/12 11:05 AM ]

fixed, http://github.com/clojure/core.match/commit/241a3a3ab5344cc7c97fb3ab0b3783ce97b09f6d





[MATCH-61] Exception thrown when matching using :seq when there is a seq call in the tail of the occurrences Created: 22/Jun/12  Updated: 22/Jun/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Emma Tosch Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bug, seq
Environment:

with Clojure 1.3



 Description   

<iframe style="width: 648px; height: 400px; border: 0px;" src="http://refheap.com/paste/3294/framed" >

I also tested with three occurrences. When there is a seq call in the second occurrence onward a RuntimeException is thrown.



 Comments   
Comment by Emma Tosch [ 22/Jun/12 5:33 PM ]

https://gist.github.com/626088b01817ac638fae
Two expressions, macro-expanded. The only difference between the expressions is that the second occurrence in the second expression is seq'ed. The second let is the one throwing the exception; it's the one with the binding

(clojure.core/let [q_tail_3472 q_tail_3472
q_head_3471 q_head_3471
ocr-3470 (seq y)
z z]
...)





[MATCH-60] Matching maps with :only broken in CLJS Created: 18/Jun/12  Updated: 01/Jul/12  Resolved: 01/Jul/12

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Kevin Lynagh Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File cljs_keyset.patch     Text File cljs_keyset.patch    
Patch: Code

 Description   

The `.keySet` method call doesn't exist on JavaScript objects.
Master throws compile errors on me, so I've attached a patch against alpha9 that removes the Java method call in favor of the pure Clojure `(set (keys m))`



 Comments   
Comment by David Nolen [ 18/Jun/12 10:49 AM ]

It would preferable to check the *clojurescript* dynamic var and do different emission. Thanks!

Comment by Kevin Lynagh [ 30/Jun/12 11:34 AM ]

Updated patch against master that uses clojurescript var. Also available at https://github.com/lynaghk/core.match/tree/60-cljs-keyset

Comment by David Nolen [ 01/Jul/12 11:03 AM ]

fixed, http://github.com/clojure/core.match/commit/cdeea55f211f1dbe4768c8aec3149bfcb00438a2





[MATCH-59] Misleading comment in clojure.core.match.bits Created: 19/May/12  Updated: 18/May/13  Resolved: 18/May/13

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Thomas Greve Kristensen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

I was looking at the code for clojure.core.match.bits, and there seems to be a reference to an example of parsing a dgram. The code looks to be broken or incomplete, or is it my understanding of it that is lacking?

If it isn't done, are there any plans to finish it up?



 Comments   
Comment by David Nolen [ 18/May/13 2:21 PM ]

The bits and array namespaces are experimental I've left a comment in both namespaces that states that.

http://github.com/clojure/core.match/commit/511b786b9545f5c00a629fa27be0ee2a6b01e2ae





[MATCH-58] we should test the presence of key with find, not get Created: 07/Apr/12  Updated: 07/Apr/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If a user really doesn't care about the existence of a key they won't match on it. The current behavior is very confusing.






[MATCH-57] Non deterministic match behavior for seqs when AOT Created: 05/Apr/12  Updated: 18/May/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Ronen Narkis Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.3

Leiningen 1.7.1 on Java 1.6.0_26

core.match "0.2.0-alpha9"



 Description   

The following source example:

https://github.com/narkisr/match-issue/blob/master/src/match_issue/core.clj

The defrule macro calls a function called lhs that use the match library,

this function calls it self recursively and print out the rest of body left to be matched,

When running lein compile two different results appear (in non deterministic fashion):

(when $message :> Message (== level 6) :from (entry-point event-stream))
($message :> Message (== level 6) :from (entry-point event-stream))
(Message (== level 6) :from (entry-point event-stream))
((== level 6) :from (entry-point event-stream))

This is the bug, as (== level 6) should be matched by:

[([([(o :when operator?) f s] :seq) :as c & r] :seq)]

In other cases the output is (the correct one):

(when $message :> Message (== level 6) :from (entry-point event-stream))
($message :> Message (== level 6) :from (entry-point event-stream))
(Message (== level 6) :from (entry-point event-stream))
((== level 6) :from (entry-point event-stream))
(:from (entry-point event-stream))
(entry-point event-stream)
()
()



 Comments   
Comment by David Nolen [ 06/Apr/12 11:03 AM ]

All AOT related issues w/o patches are low priority for the foreseeable future. They are simply too time consuming to track down and I don't have the bandwidth at the moment. I will get to them eventually, but if you want movement on this please submit a patch. I will happily apply it!

Comment by Ronen Narkis [ 07/Apr/12 7:53 PM ]

Hey David,

After going through the source code and some more in depth look I managed to understand what was going on, first iv stepped through the different stages

(def m (emit-matrix ['body]
'( [(['when & r] :seq)] (lhs r)
[(['accumulate & r] :seq)] (<< "accumulate(~(lhs r))")
[([bind ':> & r] :seq)] (<< "~{bind}:~(lhs r)"); pattern with bind
[([(t :when is-type?) & r] :seq)] (<< "{t}(lhs r)"); pattern
[([([(f :when acc-fn?) & args] :seq) & r] :seq)] (<< ",{f}{args}~(lhs r)" ); accumulate function
[([':from dest & r] :seq)] (<< "from ~(lhs dest) ~(lhs r)")
[([':over win & r] :seq)] (<< "over ~(window win) ~(lhs r)")
[([([(o :when operator?) f s] :seq) :as c & r] :seq)] (<< "(~(reduce str (map pr-str (to-infix c)))) ~(lhs r)")
[(['entry-point point & r] :seq)] (<< "entry-point \"~{point}\"~(lhs r)")
:else ""
)))

(def c (cm/compile m))

(pprint c)

(pprint (executable-form c))

The last pprint failed aot compilation:

$ lein compile

...
Caused by: java.lang.ClassCastException: clojure.core.match.WildcardPattern cannot be cast to java.lang.String

Which was very weird as I could clearly see that:

(deftype WildcardPattern [sym _meta]
IWildcardPattern
(sym [_] sym)
clojure.lang.IObj
(meta [_] _meta)
(withMeta [_ new-meta]
(WildcardPattern. sym new-meta))
Object
(toString [_]
(str sym)))

Then iv also tried to enabled trace:

(set-trace! true)

Which resulted with:

$ lein compile

...

TRACE: DAG: Column 0 : [#<SeqPattern #<LiteralPattern window> #<LiteralPattern :time> #<WildcardPattern t> #<WildcardPattern unit>>]
TRA at clojure.lang.Compiler.analyzeSeq(Compiler.java:6416)
at clojure.lang.Compiler.analyze(Compiler.java:6216)
at clojure.lang.Compiler.analyze(Compiler.java:6177)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5572)
at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5008)
at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3629)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6407)
at clojure.lang.Compiler.analyze(Compiler.java:6216)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6397)
at clojure.lang.Compiler.analyze(Compiler.java:6216)
at clojure.lang.Compiler.access$100(Compiler.java:37)
at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:492)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6409)
at clojure.lang.Compiler.analyze(Compiler.java:6216)
at clojure.lang.Compiler.analyze(Compiler.java:6177)
at clojure.lang.Compiler.compile1(Compiler.java:6980)
at clojure.lang.Compiler.compile(Compiler.java:7046)
at clojure.lang.RT.compile(RTCE: DAG: Add switch-node on occurrence exp

The most weird thing was that on second compile it all went well, this raised a flag and my suspicion was that the core.match classes got compiled on the first run:

$ ls classes/

match$analyze_actions$analyze_action__1166.class
match$analyze_actions.class
match$backtrack_expr.class
match$bind_node.class
match$catch_error.class
match$check_matrix_args$check_pattern__1150.class
...

Then iv decided to AOT compile clojure.core match:

$ git checkout core.match-0.2.0-alpha9
$ git diff project.clj
-(defproject match "0.2.0-alpha10-SNAPSHOT"
+(defproject match "0.2.0-alpha10-aot"
:description "Optimized pattern matching and predicate dispatch for Clojure"
:source-path "src/main/clojure"

  • :test-path "src/test/clojure")
    + :test-path "src/test/clojure"
    + :aot [clojure.core.match]
    + )

After using the aot jar it all went smooth including passing my non deterministic case (which is deterministic but very confusing to track down),

In order to reproduce it its important to run:

$ lein clean
$ rm -rf lib

And only then run

$ lein compile

Otherwise the classes dir might contain AOT'ed classes and make it seem that even non-aot jar works fine (its enough to run lein compile once to cause the classes to get compiled),

The fix in my case is simple, just AOT ns.

Im not sure if this Is a bug in the way Clojure AOT its classes but I think that adding the AOT'ed classes to the default core match distro is a reasonable workaround

BTW when using master (and not the tag) iv stumbled upon:

Exception in thread "main" java.lang.AssertionError: Assert failed: Unknown predicate in [is-type?]

Which is a pred defined in my ns.





[MATCH-56] IndexOutOfBoundsException when matching empty vector Created: 27/Mar/12  Updated: 27/Mar/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Greg Chapman Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Clojure 1.3


Attachments: File match.clj.diff    

 Description   

Using the latest core.match:

user=> (let [x []] (match [x] [[h & t]] [h t] :else :nomatch))
IndexOutOfBoundsException clojure.lang.RT.subvec (RT.java:1451)

Not sure if this is the best fix, but I resolved this specific case by slightly changing subvec-inline (see attached diff).



 Comments   
Comment by David Nolen [ 27/Mar/12 10:28 AM ]

Thanks for the report. The patch is not going to work - we should be checking that the vector has at least one item.





[MATCH-55] Matching a sequence with just a rest pattern fails to compile Created: 21/Mar/12  Updated: 21/Mar/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In 0.2.0-alpha9,

(match [ [1 2] ] [([& _] :seq)] true)

fails to compile with No method in multimethod 'to-source' for dispatch value: class clojure.core.match.RestPattern






[MATCH-54] Cannot AOT with certain match expression. Created: 04/Mar/12  Updated: 18/May/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jason Jackson Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If you try to AOT this code, you get an exception.

(defn -main [& args]
  (println args)
  (let [x nil] 
    (match x
      ["=" _ _] true
      [[:invoke _] _] true)))

Stack Trace:

Compiling foo.core
Exception in thread "main" java.lang.IndexOutOfBoundsException, compiling:(foo/core.clj:8)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6416)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.analyze(Compiler.java:6177)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5572)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5873)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6409)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6397)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.analyze(Compiler.java:6177)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5572)
	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5008)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3629)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6407)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6397)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.access$100(Compiler.java:37)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:492)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6409)
	at clojure.lang.Compiler.analyze(Compiler.java:6216)
	at clojure.lang.Compiler.analyze(Compiler.java:6177)
	at clojure.lang.Compiler.compile1(Compiler.java:6980)
	at clojure.lang.Compiler.compile(Compiler.java:7046)
	at clojure.lang.RT.compile(RT.java:385)
	at clojure.lang.RT.load(RT.java:425)
	at clojure.lang.RT.load(RT.java:398)
	at clojure.core$load$fn__4610.invoke(core.clj:5386)
	at clojure.core$load.doInvoke(core.clj:5385)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.core$load_one.invoke(core.clj:5200)
	at clojure.core$compile$fn__4615.invoke(core.clj:5397)
	at clojure.core$compile.invoke(core.clj:5396)
	at user$eval27.invoke(NO_SOURCE_FILE:1)
	at clojure.lang.Compiler.eval(Compiler.java:6465)
	at clojure.lang.Compiler.eval(Compiler.java:6455)
	at clojure.lang.Compiler.eval(Compiler.java:6431)
	at clojure.core$eval.invoke(core.clj:2795)
	at clojure.main$eval_opt.invoke(main.clj:296)
	at clojure.main$initialize.invoke(main.clj:315)
	at clojure.main$null_opt.invoke(main.clj:348)
	at clojure.main$main.doInvoke(main.clj:426)
	at clojure.lang.RestFn.invoke(RestFn.java:421)
	at clojure.lang.Var.invoke(Var.java:405)
	at clojure.lang.AFn.applyToHelper(AFn.java:163)
	at clojure.lang.Var.applyTo(Var.java:518)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IndexOutOfBoundsException
	at clojure.lang.PersistentVector.arrayFor(PersistentVector.java:106)
	at clojure.lang.PersistentVector.nth(PersistentVector.java:110)
	at clojure.lang.RT.nth(RT.java:741)
	at clojure.core.match.PatternRow.invoke(match.clj:327)
	at clojure.core.match.PatternMatrix.pattern_at(match.clj:713)
	at clojure.core.match$useful_p_QMARK_.invoke(match.clj:778)
	at clojure.core.match.PatternMatrix$iter__832__838$fn__839$iter__834__840$fn__841$fn__842.invoke(match.clj:735)
	at clojure.core.match.PatternMatrix$iter__832__838$fn__839$iter__834__840$fn__841.invoke(match.clj:733)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core.match.PatternMatrix$iter__832__838$fn__839.invoke(match.clj:734)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:67)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$partition$fn__3913.invoke(core.clj:2777)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$map$fn__3811.invoke(core.clj:2424)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.Cons.next(Cons.java:39)
	at clojure.lang.PersistentVector.create(PersistentVector.java:50)
	at clojure.lang.LazilyPersistentVector.create(LazilyPersistentVector.java:31)
	at clojure.core$vec.invoke(core.clj:345)
	at clojure.core.match.PatternMatrix.useful_matrix(match.clj:733)
	at clojure.core.match.PatternMatrix.necessary_column(match.clj:725)
	at clojure.core.match.PatternMatrix$choose_column__815.invoke(match.clj:690)
	at clojure.core.match.PatternMatrix.compile(match.clj:708)
	at clojure.core.match$first_column_chosen_case$switch_clauses__780$fn__781.invoke(match.clj:613)
	at clojure.core$map$fn__3815.invoke(core.clj:2437)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$reduce.invoke(core.clj:5994)
	at clojure.core$into.invoke(core.clj:6004)
	at clojure.core.match$first_column_chosen_case$switch_clauses__780.invoke(match.clj:616)
	at clojure.core.match$first_column_chosen_case.invoke(match.clj:646)
	at clojure.core.match.PatternMatrix.compile(match.clj:710)
	at clojure.core.match$first_column_chosen_case$switch_clauses__780$fn__781.invoke(match.clj:613)
	at clojure.core$map$fn__3815.invoke(core.clj:2437)
	at clojure.lang.LazySeq.sval(LazySeq.java:42)
	at clojure.lang.LazySeq.seq(LazySeq.java:60)
	at clojure.lang.RT.seq(RT.java:466)
	at clojure.core$seq.invoke(core.clj:133)
	at clojure.core$reduce.invoke(core.clj:5994)
	at clojure.core$into.invoke(core.clj:6004)
	at clojure.core.match$first_column_chosen_case$switch_clauses__780.invoke(match.clj:616)
	at clojure.core.match$first_column_chosen_case.invoke(match.clj:646)
	at clojure.core.match.PatternMatrix.compile(match.clj:710)
	at clojure.core.match$clj_form.invoke(match.clj:1616)
	at clojure.core.match$match.doInvoke(match.clj:1645)
	at clojure.lang.RestFn.invoke(RestFn.java:573)
	at clojure.lang.Var.invoke(Var.java:426)
	at clojure.lang.AFn.applyToHelper(AFn.java:193)
	at clojure.lang.Var.applyTo(Var.java:518)
	at clojure.lang.Compiler.macroexpand1(Compiler.java:6320)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6395)
	... 46 more





[MATCH-53] Match doesn't work when AOT compiled into a JAR, but manually macroexpanding and JAR'ing works fine. Created: 24/Feb/12  Updated: 18/May/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Kevin Lynagh Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Code here:

https://gist.github.com/35fa11df7d7516abff50

Running lein uberjar on source containing the offending function, geo->svg, results in a non-working application.
However, if match is manually macroexpanded in the repl and the results pasted in, then the JAR works just fine.



 Comments   
Comment by Kevin Lynagh [ 24/Feb/12 2:27 PM ]

"non-working" meaning that the match always drops directly the :else clause.

Comment by Kevin Lynagh [ 24/Feb/12 6:09 PM ]

Updated code to minimal example. Problem persists with both Lein 1.7 and cake 0.6.3

Comment by David Nolen [ 25/Feb/12 7:35 PM ]

AOT bugs are a bit tricky to track down. Not sure how soon I'll be able to really dive into this one.





[MATCH-52] Pattern Map's aren't working Created: 12/Feb/12  Updated: 14/Aug/12  Resolved: 14/Aug/12

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: Jason Jackson Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-map-matching-always-checks-for-presence-of-key-even-.patch     Text File 0001-Map-matching-should-always-check-for-presence-of-key.patch     File patch    

 Description   

Neither of these work as expected:

(match [ {:type :consumed :value 4}]
[({:uid uid :modifiers ms} :only [:uid :modifiers])] :a0)

(match [ {:type :consumed :value 4}]
[{:uid uid :modifiers ms} ] :a0)

Tried these versions:
"0.2.0-alpha10-SNAPSHOT"
"0.2.0-alpha9"



 Comments   
Comment by Jason Jackson [ 12/Feb/12 11:33 PM ]

I tried to use :when to detect when key is not found. But this doesn't work either.
(match {:foo 3}
{:uid (uid :when #(not (nil? %)))} :a0)

Comment by David Nolen [ 14/Feb/12 4:31 PM ]

Looks good, but can you create the patch so that it contains your credentials as well as test cases? Thanks!

Comment by Jason Jackson [ 13/May/12 12:06 PM ]

added unit tests and attribution info.

Comment by Kevin Lynagh [ 13/Aug/12 11:18 PM ]

I've manually applied this patch to the latest master and verified that it fixes the issue.
Patch attached and also available on the Github:

https://github.com/lynaghk/core.match/tree/issue-52

EDIT: spoke too soon; I can't seem to upload the patch to JIRA as a commenter. Available here: https://github.com/lynaghk/core.match/compare/issue-52.patch

I've also added a commit on top to get ClojureScript support.
The implementation feels a bit gross to me (it inlines the clojure core.match val-at* function, basically) but a nicer solution isn't obvious to me given ClojureScript's macro/runtime divide.

Comment by Kevin Lynagh [ 14/Aug/12 9:05 PM ]

Updated Jason Jackson's patch with CLJS support.

Comment by David Nolen [ 14/Aug/12 9:11 PM ]

Fixed, http://github.com/clojure/core.match/commit/7f73cee3f78417f1fb59bcbb4a8cda52de22efbd





[MATCH-51] Fail to match empty vector Created: 21/Jan/12  Updated: 18/May/13

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Jason Jackson Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(match (vector)
  ([(re :guard string?)] :seq) 4
  [] 6)

This should evaluate to 6 not nil.



 Comments   
Comment by Jason Jackson [ 21/Jan/12 9:30 AM ]

tested on 0.2.0-alpha10-SNAPSHOT





[MATCH-50] locals pattern matching issue Created: 12/Jan/12  Updated: 25/Feb/12  Resolved: 25/Feb/12

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   
(let [a 1]
    (match [1 2]
      [1 3] :a0
      [a 2] :a1
      :else :a2)) ;; :a2


 Comments   
Comment by David Nolen [ 25/Feb/12 6:49 PM ]

Fixed, https://github.com/clojure/core.match/commit/ac92c6df3f70f56fbe12f9d3f46585e66102c50b





[MATCH-49] Duplicate wildcard detection in pattern row doesn't account for locals Created: 12/Jan/12  Updated: 12/Jan/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None





[MATCH-48] Guards cannot be fn expressions Created: 10/Jan/12  Updated: 25/Feb/12  Resolved: 25/Feb/12

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Chris Gray Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-Added-new-multimethod-mutually-exclusive-inequality.patch     Text File 0001-Failing-test.patch     Text File 0002-Use-the-new-multimethod-and-add-some-tests.patch     File new-patches.diff    
Patch: Code and Test

 Description   

Anonymous function literals as guards (such as (a :when #(odd? %))) seem to confuse the match compiler. The attached test shows how.



 Comments   
Comment by David Nolen [ 10/Jan/12 10:04 PM ]

I'm on the fence about allowing inline fn expressions and fn literals as guards. The problem is that they can't be checked for equality and thus tests cannot be shared across guard patterns. I need to think about it some more but I don't consider this high priority in the near future. Any decision will have to take into consideration the goal of predicate dispatch.

Comment by Chris Gray [ 11/Jan/12 12:41 PM ]

Sorry, I think the fact that the functions in my earlier example were function literals was a bit of a red herring. The following test also fails.

(deftest guard-pattern-match-5
  (is (=
       (let [oddp odd?]
         (match [1 2]
                [a :when odd? b :when odd?] :a1
                [a :when oddp _] :a2
                [_ b :when even?] :a3
                :else :a4))
       :a2)))
Comment by David Nolen [ 11/Jan/12 12:44 PM ]

The earlier examples where not a red herring. This is likely a separate issue.

Comment by Chris Gray [ 11/Jan/12 12:50 PM ]

I really don't see how, given that there seems to be no code that specializes on the type of function given to a guard. My guess is that when guard-pattern-match-5 succeeds, guard-pattern-match-4 will succeed as well.

Comment by David Nolen [ 11/Jan/12 1:00 PM ]

Oh sorry you are right. This is exactly same problem. We can't know that odd? and oddp are the same. Again this is not something I'm interested in fixing without a lot more consideration.

Basically functions can't be tested for equality like types can. This means that the presence of a guard must create a backtrack point. However if we make guards work a little more like types (you have to declare them ahead of time) we lose a little bit of convenience but gain a lot of reasoning power and can share tests and avoid these pitfalls.

This discussion probably needs a design page.

Comment by Chris Gray [ 11/Jan/12 6:11 PM ]

Two more examples of the same problem, this time not using guards:

(deftest unequal-equal-tests
  (is (=
       (match ["foo" "bar"]
              [#".*" #"baz"] :a1
              [#"foo" _] :a2
              [_ "bar"] :a3
              :else :a4)
       :a2)))

(deftest unequal-equal-tests-2
  (is (=
       (let [a 1]
        (match [1 2]
               [(:or 1 2) 3] :a1
               [(:or 2 a) 2] :a2
               [1 _] :a3
               :else :a4))
       :a2)))
Comment by David Nolen [ 11/Jan/12 6:58 PM ]

They are not the same problem. These don't work for very different reasons. The first I probably won't even address and will leave for the community to deal with - I don't need robust pattern matching on regexes.

The second example is a legitimate bug around matching locals which is unrelated to this ticket. Feel free to open a new one for it.

Comment by Chris Gray [ 11/Jan/12 7:15 PM ]

Yes, you're right, the second is unrelated.

Comment by Chris Gray [ 12/Jan/12 11:20 AM ]

On further reflection, what all these examples show is that Maranget's algorithm is only correct for literals whose equality you can test at compile time. Thus, not even locals will work using his algorithm. Regexes and functions will certainly not work correctly 100% of the time.

What happens is that when multiple unequal tests in the same column can return a truthy value, you end up with a decision forest rather than a decision tree. If the first decision tree in the forest has the first and third end-states, while the second tree has the second end-state, if you end up in the third end-state, you must still check the second decision tree before you decide which end-state is actually correct.

This is a shame, since it means that compiling the matches becomes more complex. On the other hand, it seems like a great subject for a paper at programming-language conference, so there's always that.

On a serious note, though, this bug is major, and you should consider removing support for at least guards, locals, and regexes until it is fixed. The bugs that arise from it in the end-user's code are really hard to track down – it's as if `or` or `and` were broken 10% of the time.

Comment by David Nolen [ 12/Jan/12 11:29 AM ]

There is nothing wrong with Maranget's algorithm. We just have to be sure that we create a backtrack point - that's all.

Functions cannot be fixed because function equality is undecideable. So for guards we might create a backtrack point. I've already updated the README to describe what works at the moment. I have a branch which throws an error if :when is not given a vector of symbols or a symbol. It should probably be improved so that symbols are known top levels (no reassigning fns to locals).

Regex equality can probably be made to work but I'm not going to do it. (On further thought we can probably make patterns create backtrack points by default, can be overridden for those willing to make their patterns highly optimizeable)

Locals can be fixed, we'll definitely create a backtrack point for these.

Comment by Chris Gray [ 12/Jan/12 11:54 AM ]

I'm sorry, but this problem will not be solved by backtracking alone. At least not with the backtracking mechanism that currently exists.

With backtracking, you are still treating the problem as though you have a decision tree. A decision tree requires that all the tests at its nodes are mutually exclusive.

By assuming that you have a decision tree, once a match is found in the tree, that match is returned. As my last comment pointed out, that is not sufficient. You must also check to see if there is a match in an end-state that was declared earlier. I really don't see that that's possible given the current backtracking system.

Comment by David Nolen [ 12/Jan/12 1:26 PM ]

I don't follow you. Maranget's algorithm is not sufficient for pattern matching if we don't constrain columns to specific types. There are many things already in place to deal with the shortcomings of Maranget's approach given what we want to accomplish - for example, we actually have a grouping pass. This is the same approach that Racket uses as far as I can tell and they don't have any problems. Certainly none of the patterns you shown thus that far (besides fns exprs) pose any challenges that I can see.

Comment by Chris Gray [ 12/Jan/12 1:42 PM ]

Sorry, I was editing my comment as you made yours. I hope it is more clear now.

I guess I don't totally understand your code still, so I will try to rectify that before commenting again. From what I have seen, though, you are trying to build a decision tree. What I am saying is that isn't possible at compile time, since you can't ensure that the nodes of the tree are mutually exclusive.

Comment by David Nolen [ 12/Jan/12 1:51 PM ]

All known constructors are considered mutually exclusive. We group all constructors in a column preserving order as closely as possible. Decisions trees are created for these constructors. If we cannot know at compile time whether something is mutually exclusive (wildcards, locals), we create a backtrack point to handle them.

Consider that if we only have backtrack points (no trees) all tests could potentially be tried. Our approach is a hybrid one - we don't rely only on decisions trees and we don't rely only on backtracking.

When we get to integrating pattern matching on interfaces, protocols ambiguities of course become possibly. But even this can probably be handled reasonably with something like "prefers".

Comment by Chris Gray [ 12/Jan/12 3:07 PM ]

Consider the following pair of decision trees:

  1
 /
a
 \
  3*

  2*
 /
b--4*
 \
  5*

Where the numbers are the order in which the terminals appear, and they have stars beside them if they match. The correct terminal for core.match to return in this case is the second one. Currently, the code would return the third terminal. Suppose, however, that backtracking was added so that the tree rooted at b was checked for matches as well. This is certainly possible, though a lot of information would need to be kept about the match already found (which is what I meant about things not working with the current backtracking system). Also, you must ensure that the testing stops when you hit the second terminal, for two reasons – first, not doing so would imply that all terminals are checked, and second, the test to distinguish 4 from 5 could throw an exception. For similar reasons, the return value of the third terminal can't be computed – it could be a very long computation, or it could throw an exception.

Comment by David Nolen [ 12/Jan/12 3:28 PM ]

If the correct terminal is the second one, we will return the second one. No information needs to be kept around. I suggest you take a closer look at the code at this point.

Comment by Chris Gray [ 12/Jan/12 6:58 PM ]

Aah, you're right. (I think.) Might it be more accurate to say that the situation I proposed can't happen? That is, no two trees are created where there are lower-numbered terminals in the second tree than in the first tree?

Is the plan to add backtrack points for everything where equality can't be determined at compile time?

Comment by Chris Gray [ 12/Jan/12 11:27 PM ]

These patches implement the proper backtracking for tests that are not mutually exclusive.

Comment by David Nolen [ 13/Jan/12 9:15 AM ]

Wow, this is great. I've skimmed over the patches and they look pretty good. I will go over them more closely as soon as I can - there are a couple changes we should probably make. Thanks!

Comment by Chris Gray [ 13/Jan/12 11:08 AM ]

No problem. My apologies for the persistent misunderstanding.

Comment by David Nolen [ 14/Feb/12 4:39 PM ]

Sorry for the epic delay. Here are my notes on that patches:

1. Let's rename mutually-exclusive-inequality? to comparable?
2. I want GuardPatterns to comparable, I'm not going to support arbitrary fns - not worth it IMO.
As a compromise I'd be willing to rename :when patterns -> PredicatePatterns and GuardPatterns
can be allowed via :guard.
3. No need to declare OrPatterns as comparable - they aren't real patterns
4. No need to exclude VectorPatterns - the way that pattern matching works makes this unnecessary

If you make these changes I promise to apply these in a timely manner

Comment by Chris Gray [ 15/Feb/12 12:50 PM ]

Okay, patch uploaded. There is a fairly significant portion of it that is just cut and paste, which I'm not so happy about, but I don't think there's a way to do type inheritance, so I did the easier thing.

Comment by David Nolen [ 25/Feb/12 6:49 PM ]

Fixed, https://github.com/clojure/core.match/commit/ac92c6df3f70f56fbe12f9d3f46585e66102c50b

addressed MATCH-50





[MATCH-47] vector patterns dispatch on count after dispatching on type Created: 23/Dec/11  Updated: 23/Dec/11

Status: In Progress
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

The current behavior just creates complications.






[MATCH-46] :or leaks into the matches Created: 22/Dec/11  Updated: 22/Dec/11  Resolved: 22/Dec/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

Macroexpanding an match expression with an :or pattern will show this.



 Comments   
Comment by David Nolen [ 22/Dec/11 12:52 AM ]

Fixed, https://github.com/clojure/core.match/commit/c1430c98937f31a0c8d2a92d793b0795b2c9a1d6





[MATCH-45] group types together Created: 21/Dec/11  Updated: 23/Dec/11  Resolved: 23/Dec/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently if patterns of the same type are not adjacent, matching fails unexpectedly. This should always work. This does beg the question about things which might possible match multiple clauses. When we get to predicate dispatch we may check for subsumption. However in some cases we might have something which is legitimately both things. We haven't decided yet how we'll handle that.



 Comments   
Comment by David Nolen [ 23/Dec/11 10:09 PM ]

Fixed, https://github.com/clojure/core.match/commit/29607a2105d8af90c5b8d9d4cde9191e63a2570c





[MATCH-44] regroup-keywords should not use gensym Created: 12/Dec/11  Updated: 12/Dec/11  Resolved: 12/Dec/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File 0001-improved-regroup-keywords.patch    
Patch: Code

 Description   

I contributed the regroup-keywords function to allow 'flattened' match syntax for :when and :as. It used gensym to make a marker to simplify the algorithm. After seeing how other people had done similar things with sentinel values, I realized that (Object.) is a better unique value. Theoretically, an evil user could use the same symbol that the gensym had created. Also, it's better to test with identical? rather than = since the sentinel is unique. I will attach a patch with a slight refactoring.



 Comments   
Comment by David Nolen [ 12/Dec/11 9:30 PM ]

Fixed, https://github.com/clojure/core.match/commit/6d0f3fe33c4a85a12366d447e82cab59e299f94a





[MATCH-43] Vector pattern - unreachable clause Created: 10/Dec/11  Updated: 27/Dec/11  Resolved: 27/Dec/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Benny Tsai Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Clojure 1.3, core.match 0.2.0-alpha8



 Description   

This is the simplest example I could come up with:

(defn f [xs]
(match xs
[:a] "a"
[:b b] b
[:c] "c"
:else "problem!"))

[:a] and [:b b] can be matched with no problems, but [:c] can't be matched for some reason:

user=> (f [:a])
"a"
user=> (f [:b 1])
1
user=> (f [:c])
"problem!"



 Comments   
Comment by David Nolen [ 12/Dec/11 9:24 PM ]

This will have to wait for http://dev.clojure.org/jira/browse/MATCH-31. There are some deeper issues with vector pattern matching that need to get ironed out first.

In the meantime just put your [:c] test above [:b b]. The key idea is to keep vector patterns of the same size "together".

Comment by David Nolen [ 27/Dec/11 8:23 PM ]

fixed, https://github.com/clojure/core.match/commit/b2ee29d701a9306c1c494d91c371c01a512aee0c





[MATCH-42] quoted symbols should be treated as literals Created: 30/Nov/11  Updated: 30/Nov/11  Resolved: 30/Nov/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Steve Miner Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File match-42-quoted-symbol.patch    

 Description   

Reported on the Clojure mailing list:

From: Alex Miller <alex@puredanger.com>
Subject: Symbol matching in core.match
Date: November 30, 2011 2:49:43 PM EST
To: Clojure <clojure@googlegroups.com>
Reply-To: clojure@googlegroups.com

I've been working with core.match some this week and finding it pretty
nice. However, I have a common case (for me) that is not well handled
right now via core.match: matching symbols. Say that I wrote a match
like this:

;; translate (+ x (+ y z)) to (+ x y z)
(let [e '(+ 1 (+ 2 3))]
(match [e]
[([+ x ([+ y z] :seq)] :seq)] (+ x y z)))

You will see this error:
Pattern row 1: Pattern row reuses wildcards in [([+ x ([+ y
z] :seq)] :seq)]. The following wildcards are ambiguous: +. There's
no guarantee that the matched values will be same. Rename the
occurrences uniquely.

Any symbol inside a pattern row is treated as a bind variable. + is a
symbol. You can achieve this with guards:

(defn +? [s] (= '+ s))

(let [e '(+ 1 (+ 2 3))]
(match ['(+ 1 (+ 2 3))]
[([(_o1 :when +?) x ([(_o2 :when +?) y z] :seq)] :seq)] (list
'+ x y z)))

but, yuck. I can imagine using the reserved ()'s with additional keys
(:symbol or :sym) to do symbol matching like (:symbol +) but also,
yuck. The simplest idea I came up with was:

(let [e '(+ 1 (+ 2 3))]
(match [e]
[(['+ x (['+ y z] :seq)] :seq)] ('+ x y z)))

These come through as (quote x) although the error reporting goes a
little off the rails:
Pattern row 1: Pattern row reuses wildcards in [([(quote +) x ([(quote
+) y z] :seq)] :seq)]. The following wildcards are ambiguous: quote.
There's no guarantee that the matched values will be same. Rename the
occurrences uniquely.

However, that seems fixable and you could then use (quote x) as a
signal to do symbol matching. If I can figure out what the hell I'm
doing in core.match then I'd be happy to work on a patch.



 Comments   
Comment by Steve Miner [ 30/Nov/11 4:56 PM ]

Skip anything that's quoted when looking for duplicate symbol names. Added a test for the reported case.

Comment by Steve Miner [ 30/Nov/11 4:57 PM ]

patch attached

Comment by David Nolen [ 30/Nov/11 6:52 PM ]

Fixed, https://github.com/clojure/core.match/commit/6721be4fba74561038539e12667bc04cc5fc94cc





[MATCH-41] This 5-clause match expression fails to compile Created: 30/Nov/11  Updated: 30/Nov/11  Resolved: 30/Nov/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Gary Fredericks Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

Ubuntu 10.10

java version "1.6.0_20"
OpenJDK Runtime Environment (IcedTea6 1.9.10) (6b20-1.9.10-0ubuntu1~10.10.2)
OpenJDK 64-Bit Server VM (build 19.0-b09, mixed mode)



 Description   

This expression fails to compile with a NullPointerException. Oddly enough I seem unable to make it any more minimal:

(match [["foo"]]
    [["foo"]] :a0
    [["foo" a]] :a1
    [["baz"]] :a2
    [["baz" a b]] :a3)

When I included the SNAPSHOT release in my project that seemed to fix it, but oddly enough I cloned the git repo this morning and it was still broken on HEAD, so I have no idea what's going on.



 Comments   
Comment by David Nolen [ 30/Nov/11 8:12 PM ]

Fixed, https://github.com/clojure/core.match/commit/a790f7900da9152dcdf290ade34b8001c47869f1





[MATCH-40] Allow or'ing of guard functions Created: 21/Nov/11  Updated: 21/Nov/11

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In order to simplify composition of guard predicates which are alternatives, allow for passing a sequence of predicates whose values when called will be or'd together.

The :when keyword currently allows passing of a vector of predicates that are and'd together.

Either case (`and` or `or`) can be achieved externally to match via composition, and an alternative might be to force explicit composition outside of core.match.

At the least, the documentation should mention that multiple predicates will be and'd together.






[MATCH-39] Allow matching of map expressions to restrict the set of keys present in the value to a subset of a specified set of keys Created: 21/Nov/11  Updated: 21/Nov/11

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It is useful, to be able to constrain a match on a map value to limit the set of keys present in the value, without enforcing that the keys are all present in the value.

The current :only option, enforces a strict matching of keys.

{{(let [x {:a 1} (match [x] [({:a _ :b _} :only [:a :b])])}} => doesn't match

This came up trying to write a precondition on the argument of a function, which allowed optional keys, but wanted to restrict the overall set of keys.



 Comments   
Comment by David Nolen [ 21/Nov/11 12:22 PM ]
(match x
  ({:c 1} :has [:a :b]) :a0
  ...)

or

(match x
  ({:c 1} :contains [:a :b]) :a0
  ...)

Seems reasonable.

Comment by David Nolen [ 21/Nov/11 12:34 PM ]

Ah misunderstood. You want something like :allowed.





[MATCH-38] interpose1 no longer needed Created: 29/Oct/11  Updated: 29/Oct/11  Resolved: 29/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

With the fix for MATCH-34, the function interpose1 is no longer used and can be removed. It was there only to support the old infix OR pattern.



 Comments   
Comment by David Nolen [ 29/Oct/11 12:50 PM ]

Fixed, https://github.com/clojure/core.match/commit/314c665197bf6f74ed5e66518e93b48f5778ff60





[MATCH-37] remove match-1, match should support match-1 behavior Created: 28/Oct/11  Updated: 28/Oct/11  Resolved: 28/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

matching literal vectors is not something we support so there can be no ambiguity.



 Comments   
Comment by David Nolen [ 28/Oct/11 12:31 AM ]

Fixed, https://github.com/clojure/core.match/commit/5edc28c6cd5315818ec2a852e931f5b7fe7dff35





[MATCH-36] throw on unsuccessful match Created: 27/Oct/11  Updated: 30/Nov/11

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

I was on the fence about this. But after looking at the literature as well as the behavior of condp, I've decided that throwing on unsuccessful match is the way to go. This is particularly important since we put no constraints on the types allowed - we cannot determine exhaustiveness.






[MATCH-35] Bug in seq pattern matching Created: 27/Oct/11  Updated: 11/Jan/12

Status: Open
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   
(let [l '(1 2 3)]
    (match [l]
      [([a & [b & [c d]]] :seq)] :a0
      :else :a1))

Matches when it shouldn't.



 Comments   
Comment by Greg Chapman [ 11/Jan/12 10:24 AM ]

Another example of (I think) the same issue:

user=> (let [x ()] (match [x] [([h & t] :seq)] [h t] [_] :a1))
[nil ()]

Perhaps SeqPattern's IPatternCompile should call seq in order to filter empty seqs? (e.g.:

(to-source* [this ocr]
`(and (or (seq? ~ocr) (sequential? ~ocr)) (seq ~ocr)))





[MATCH-34] remove infix or pattern syntax, use (:or x y z) instead Created: 21/Oct/11  Updated: 27/Oct/11  Resolved: 27/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 1
Labels: None


 Comments   
Comment by David Nolen [ 27/Oct/11 11:17 PM ]

Fixed, https://github.com/clojure/core.match/commit/0195f7211d44e15d40a35445b4882fcaf5bc9fb6





[MATCH-33] typo in README.md Created: 20/Oct/11  Updated: 27/Oct/11  Resolved: 27/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Steve Miner Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

Re: "As shown, :else clauses are special, in that they are not implicitely wrapped in []."

"implicitely" is misspelled, but "implicitly" would be the wrong word anyway as the intended meaning seems to be "explicitly". It would be clearer just to leave out the adverb.



 Comments   
Comment by David Nolen [ 27/Oct/11 11:19 PM ]

Fixed, https://github.com/clojure/core.match/commit/1399a77e7833abb3a41be538a6356bc3bd871152





[MATCH-32] Allow flattened pattern syntax at top-level to avoid nesting :when and :as Created: 12/Oct/11  Updated: 21/Oct/11  Resolved: 21/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Steve Miner Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File flat2.patch     Text File flat3.patch     Text File flat4.patch     Text File flat.patch    
Patch: Accepted
Approval: Ok

 Description   

On the Clojure mailing list I wrote:

For guards, I wonder if the extra parens are really necessary. Couldn't the :when bind tightly to the previous pattern variable? Like Clojure FOR comprehensions. I think it would be easier to read that way. Same comment applies to :as. To cover the rare case of matching a literal :when or :as, you could quote it or use it as the first item in an OR pattern.

On Oct 10, 2011, at 3:11 PM, David Nolen wrote:

It would be nice to lose the extra parens - patch welcome



 Comments   
Comment by Steve Miner [ 12/Oct/11 10:35 AM ]

I attached a patch that groups the :when and :as terms appropriately given a "flattened" syntax. That is, [a :when even?] becomes [(a :when even?)] before being processed as usual. I only applied it to the top-level of the pattern row because I was concerned about recursive performance and things can get pretty complicated so maybe the magic isn't worth it.

Only :when and :as at the top level get any special treatment. No mangling for :seq or ::vector, etc. No mangling of nested patterns.

I also added a couple of tests for my changes.

Comment by Steve Miner [ 14/Oct/11 1:03 PM ]

I wrote an improved patch that should handle nested patterns as well. Still only groups :when and :as, not :seq since that seems sort of special.

Comment by Steve Miner [ 15/Oct/11 7:59 AM ]

Better to use seq? instead of list?

Comment by David Nolen [ 15/Oct/11 10:24 AM ]

At first glance this looks pretty neat! Will try to take a deeper look today. Thanks!

Comment by David Nolen [ 15/Oct/11 10:47 AM ]

One issue I have with this syntax enhancement - how does one match :when or :as as literals?

Comment by Steve Miner [ 15/Oct/11 3:13 PM ]

The patch tries to group the :when or :as if that makes sense. If it doesn't make sense [:when true nil], it leaves it alone so that would match the literal. In the rare case where it would be ambiguous, you would have to use a single OR pattern to get a literal match, such as [a (:when |) false]. I put something like that in one of my new tests.

Comment by Steve Miner [ 15/Oct/11 3:18 PM ]

By the way, it might be nice to allow a single element list to match that single pattern. That is, treat [(p)] the same as [p]. Right now, it's considered an error. It would also simply my patch, where I had to introduce interpose1 (as a variant of interpose) to handle an edge case with the OR pattern.

Comment by David Nolen [ 20/Oct/11 1:54 PM ]

I'm not a fan of (:when |). I'd prefer it if the patch included support for always matching literal keywords that are written as ':foo.

Comment by Steve Miner [ 20/Oct/11 3:04 PM ]

Revised patch to support ':when (and quoted keywords in general) to match the literal keyword. Updated against HEAD.

Comment by David Nolen [ 21/Oct/11 6:53 PM ]

Fixed, https://github.com/clojure/core.match/commit/d69abfaf68a86d5b9d73070e666720770779118f





[MATCH-31] vector patterns should work on seq Created: 10/Oct/11  Updated: 30/Nov/11

Status: In Progress
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Critical
Reporter: David Nolen Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: None


 Description   

This means abandoning subvec and using nth + offsets.






[MATCH-30] throw if binding name reused in the same pattern row Created: 10/Oct/11  Updated: 19/Oct/11  Resolved: 19/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File dup2.patch     Text File dup3.patch     Text File dup4.patch     Text File dup.patch    
Patch: Accepted
Approval: Ok

 Comments   
Comment by Steve Miner [ 15/Oct/11 3:43 PM ]

Checks for duplicate wildcard names in a pattern row.

Comment by David Nolen [ 15/Oct/11 4:05 PM ]

You should be able to reuse bindings as is done in the red black tree pattern as there is no ambiguity there. If we can get that enhancement in I'll gladly accept this. Thanks for taking this on!

Comment by Steve Miner [ 16/Oct/11 3:53 PM ]

The new patch allows OR patterns to reuse wildcards, but still complains if there are other ambiguities. I added a couple of tests to illustrate.

Comment by David Nolen [ 17/Oct/11 10:33 AM ]

This looks good. It looks like you created this patch before I added your match-let. Mind creating a new version of this patch against HEAD? Thanks much.

Comment by Steve Miner [ 18/Oct/11 2:47 PM ]

Updated patch for latest HEAD

Comment by David Nolen [ 18/Oct/11 7:14 PM ]

I'm trying to apply with patch with git am, but I keep getting:

Applying: * src/main/clojure/clojure/core/match.clj: Add Stephen Miner's match-let
error: patch failed: src/main/clojure/clojure/core/match.clj:1532
error: src/main/clojure/clojure/core/match.clj: patch does not apply
Patch failed at 0001 * src/main/clojure/clojure/core/match.clj: Add Stephen Miner's match-let

It looks like the previous commit of my pasting of your match-let patch is mixed up into your latest patch.

Comment by Steve Miner [ 19/Oct/11 7:22 AM ]

Sorry, I made a git mistake with dup3.patch. I made a fresh patch called dup4.patch that should fix the problem.

Comment by David Nolen [ 19/Oct/11 9:57 AM ]

Fixed https://github.com/clojure/core.match/commit/819f7f7d874f63b74ba634a9bebc95da574d80eb





[MATCH-29] match should not throw, return nil if no match found like cond Created: 09/Oct/11  Updated: 10/Oct/11  Resolved: 10/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 10/Oct/11 10:55 PM ]

Fixed, https://github.com/clojure/core.match/commit/c9e47acfc7dd4364428cfe011476a3492787f050





[MATCH-28] clojure.core.match ns instead of clojure.core.match.core Created: 09/Oct/11  Updated: 10/Oct/11  Resolved: 10/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 10/Oct/11 10:34 PM ]

Fixed, https://github.com/clojure/core.match/commit/af647a77415f5a8c8cc3d1222873b55f91241dde





[MATCH-27] match eats exceptions Created: 06/Oct/11  Updated: 06/Oct/11  Resolved: 06/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: mindslight Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None
Environment:

match 0.2.0-alpha4, clojure 1.2.0



 Description   

user=> (match-1 :a :a (throw (Exception.)) :else :c)
:c

I would expect the exception to flow through the match. Instead, it gets eaten and one is left wondering why the proper clause "isn't matching".



 Comments   
Comment by David Nolen [ 06/Oct/11 10:51 PM ]

Fixed, https://github.com/clojure/core.match/commit/94f240dee09434ab87d2f901c647fdbd73792e89





[MATCH-26] Bug in the way that constructor set for a column is computed Created: 05/Oct/11  Updated: 09/Oct/11  Resolved: 09/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   
(let [x '(1 2) y 1]
  (match [x y]
    [([1] :seq) _] :a0
    [_ 1] :a1
    [([1 2] :seq) _] :a2
    [_ 2] :a3
    :else :a4))

This returns :a2 when it should return :a1



 Comments   
Comment by David Nolen [ 09/Oct/11 2:22 PM ]

Fixed, https://github.com/clojure/core.match/commit/efa880320fc235536690ba8af078a59c17f3321d





[MATCH-25] Map pattern :only case still not right Created: 05/Oct/11  Updated: 06/Oct/11  Resolved: 06/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   
(let [m {:a 1 :b 2}]
  (match [m]
    [({:a 1} :only [:a])] :a0
    :else :a1))

We get :a0 instead of :a1



 Comments   
Comment by David Nolen [ 06/Oct/11 8:28 PM ]

Fixed, https://github.com/clojure/core.match/commit/3407090009b447b594a7c361c8891f9b6e8f72bb





[MATCH-24] Setting *warn-on-reflection* affects all code using core.match Created: 03/Oct/11  Updated: 03/Oct/11  Resolved: 03/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   

Setting warn-on-reflection affects all use of core.match.

Please remove all cases where warn-on-reflection is set.

This can be set by the pom for the project, rather than in source, so it doesn't affect code using core.match.



 Comments   
Comment by David Nolen [ 03/Oct/11 7:38 AM ]

Fixed, https://github.com/clojure/core.match/commit/f75152edb697cff9bbe9070478b5bc0105f140ca

Do you have an example of setting it in the POM?

Comment by Hugo Duncan [ 03/Oct/11 11:32 AM ]

Thanks!

I assume the clojure build is using clojure-maven-plugin, in which case it is just a matter of setting warnOnReflection. Something like this (untested):

<build>
 <plugins>
  <plugin>
    <groupId>com.theoryinpractise</groupId>
    <artifactId>clojure-maven-plugin</artifactId>
    <configuration>
      <warnOnReflection>true</warnOnReflection>
    </configuration>
  </plugin>
 </plugins>
</build>




[MATCH-23] map pattern order bug Created: 03/Oct/11  Updated: 05/Oct/11  Resolved: 05/Oct/11

Status: Resolved
Project: core.match
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: