Patch clj-1204-make-hash-consistent-with-equal-for-bigintegers-v1.txt dated Apr 18 2013 takes the following approach to the issue:

Change the behavior of hasheq so that when given a BigInteger value that could fit into a long, returns the same hash code as that long value.

hasheq continues to return x.hashCode() if the BigInteger value does not fit into a long. This is consistent with the hash value returned by a BigInt value that does not fit into a long.

New tests are included, some of which fail without the change to hasheq, but pass with the change.

Overwrite patch with one that leaves out some unnecessary code.

Changing priority to minor, since I suppose one could work around this issue, if you were diligent about it, by converting all BigIntegers to BigInts before they are ever used in a place where they are hashed.

Mac os X Clojure 1.5.1

Patch clj-1197-make-bigint-work-on-all-doubles-v1.txt dated Apr 7 2013 changes bigint and biginteger so that they return the correct value for all float and double values, and no longer throw an exception.

Looks good. Tests pass and the failing example now converts correctly

Path clj-1191-patch-v1.txt enhances apropos to put a namespace/ qualifier before every symbol found that is not in the current namespace ns. It also finds the shortest namespace alias if there is more than one. Examples of output with patch:

user=> (apropos "replace")
(replace clojure.string/re-quote-replacement clojure.string/replace clojure.string/replace-first clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

user=> (require '[clojure.string :as str])
nil
user=> (apropos "replace")
(replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace str/re-quote-replacement str/replace str/replace-first)

user=> (in-ns 'clojure.string)
#<Namespace clojure.string>
clojure.string=> (clojure.repl/apropos "replace")
(re-quote-replacement replace replace-by replace-first replace-first-by replace-first-char replace-first-str clojure.core/replace clojure.walk/postwalk-replace clojure.walk/prewalk-replace)

+1

apropos as it already stands is quite helpful for already-referred vars, but not for vars that are only in other nses.

This update includes the information someone would need to further investigate the output.

After patch:

user=> (meta '^:foo [])
{:foo true}
user=> (meta '^:foo [:a])
{:foo true}
user=> (def ^:const foo ^:foo [])
#'user/foo
user=> (meta foo)
{:foo true}

I believe the title should read "Clojure loses quoted metdata on empty literals".

At first glance, the implementation looks wrong in that it blocks non-IObjs ever getting to EmptyExpr. Probably all persistent collections are IObjs, but would not want to bake this in.

You're right, I've updated my patch, it should work as expected now

Nicola: Your updated patch 001-CLJ-1187.patch dated Mar 29, 2013 gives syntax errors when I try to compile it.

Oh, the irony, I messed up some parentheses writing java.

Sorry for it, here's the correct patch, it applies on upstream/master.

Attaching patch

added an implementation for the equiv method if both args are java.util.regex.Pattern

Omer, could you also include in your patch a few test cases? At least one that validates that two regex's that should be equal, are equal, and another that two regex's that should be different, are non-equal. Preferably the first of those tests should fail with the existing Clojure code, and pass with your changes.

I updated the patch with some tests. Hope I added them in the correct file. I also changed the implementation a bit, by instead of adding another implementation of equiv with a different signature, checking the type of the Object in the equiv method with signature (Object k1, Object k2).
For the sake of consistency I also added an EquivPred implementation, though I'm not entirely sure when its used.

All comments here refer to the patch named fix-CLJ-1182.diff dated Mar 13, 2013.

The location for the new tests looks reasonable. However, note that your new patch has your old changes plus some new ones, not just the new ones. In particular, the new signature for equiv is still in your latest patch. You should start from a clean pull of the latest Clojure master and make only the changes you want when creating a patch, not build on top of previous changes you have made.

Also, there are several whitespace-only changes in your patch that should not be included.

I uploaded a clean patch, removing the whitespace diff and only adding the latest changes. Thanks for clarifying the workflow!
Just to clarify, this refers to the patch named fix-CLJ-1182.diff dated Mar 13 1:34 PM

I am recategorizing this as an enhancement, because if this is a bug it is a bug in Java – the Java Patterns class documents being immutable, but apparently does not implement .equals.

Other recent "make Clojure more complicated to work around problems in Java" patches have been rejected, and I suspect this one will be too, because it might impact the performance of equality everywhere.

At first pass, Rich and I both believe that, as regex equality is undecidable, that Java made the right choice in using identity for equality, that this ticket should be declined, and the ClojureScript should revert to match.

But leaving this ticket open for now so that ClojureScript dev can weigh in.

What do you mean when you say "undecidable"?

If Regex instances were interned by the reader, but still used identity for equality, then code like

(= #"abc" #"abc")

would return true, but it wouldn't diverge in the definition of equality for regular expressions between Java and Clojure.

Undecidable means that for any given regular expression, there is no single way to write it. For example #"[a]" #"a" both match the same strings, but are they the same? Maybe. But how can we decide if /any/ given regular expression matches all of the same strings as any other? The answer is, you can't. Java does provide a Pattern#pattern method that returns the string that was used to build it, but I'm not sure if that could/should be used as a basis for equality given the potential perf hit.

I posted in Stu's thread: https://groups.google.com/d/topic/clojure-dev/OTPNJQbPtds/discussion
TL;DR: Disagree with undecidability, agree with reverting to identity based equality

That makes sense to me. Thanks Fogus.

From my post to the ml thread, it might not be entirely clear, why I don't think we should implement equality for host regexes:

It would involve parsing and would leave a lot of room for errors and platform-idiosycracies to leak. And it would be different for every platform.

As Alexander said, I also think this ticket could be resolved by interning regex literals, akin to keywords. That, however, would warrant a design page first, because there are tradeoffs to be made about how far the interning should go.

Why are we spending time on this? Where is the problem statement? Does someone actually need this for a real world purpose not solved by using regex strings as keys?

It was prompted as a matter of consistency, which I think is valid. I can't think of a good reason to use regex's as keys though.

Below is a workaround, at least. I don't know, but perhaps the as-file method for URLs in io.clj of Clojure, the part that converts %hh sequences to a character with code point in the range 0 through 255, is at least partly at fault here. I don't know right now if it is possible to modify that code to handle the general case of whatever character encoding munging is going on here to when .getResource creates the URL object.

clojure.java.io/resource is documented to return a Java object of type java.net.URL, which seems like it does %hh escaping of many characters. Reference [1] to a Java bug from 2001 where a Java user was surprised by the then-recent change in behavior of the getResource method [2].

Doing a little searching I found this StackOverflow question [3], which has what might be a workaround. I tried it on my Mac OS X 10.6 system running JDK 1.6 and it seemed to work:

(slurp (.getContent (clojure.java.io/resource "abcíd/foo.txt")))

That getContent is a method for class java.net.URL [4]

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4466485
[2] http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/Class.html#getResource%28java.lang.String%29
[3] http://stackoverflow.com/questions/13013629/best-international-alternative-to-javas-getclass-getresource
[4] http://docs.oracle.com/javase/1.5.0/docs/api/java/net/URL.html#getContent%28%29

Hi Andy,

Thanks for the background and suggestions, that's very helpful.

I'm gradually learning Clojure with no Java experience. In this case I was searching for the preferred Clojure way to access items in directories declared under :resource-paths in a Leiningen project.clj file. Perhaps clojure.java.io/resource isn't the best way to do this as it's possibly too tied to the expectation for a URI instead of a more general IRI.

You're suggested workaround did work for my use case:

(slurp (.getContent (clojure.java.io/resource "abcíd/foo.txt")))

but hopefully there would be more native/direct Clojure way to accomplish the same eventually.

I don't know if java.net.IDN would be useful internally as a fix in clojure.java.io/resource — I'm assuming not since it wasn't added until Java 6.[1]

user=> (import 'java.net.IDN)
java.net.IDN
user=> (java.net.IDN/toASCII "/dir/déf")
"xn--/dir/df-gya"
user=> (java.net.IDN/toUnicode "xn--/dir/df-gya")
"/dir/déf"

[1]: http://docs.oracle.com/javase/6/docs/api/java/net/IDN.html

Patch clj-1177-patch-v1.txt dated Mar 8 2013 is an attempt to solve this issue, in what I think may be a correct way. As specified in RFC 3986, when taking a Unicode string and making a URL of it, it should be encoded in UTF-8 and then each individual byte is subject to the %HH hex encoding. This patch reverses that to turn URLs into file names.

Tested on Mac OS X 10.6 with a command line like this (it doesn't work without the -Dfile.encoding=UTF-8 option on my Mac, probably because the default encoding is MacRoman):

% java -cp clojure.jar:path/to/resource -Dfile.encoding=UTF-8 clojure.main
user=> (require '[clojure.java.io :as io])
nil
user=> (io/resource "abcíd/foo.txt")
#<URL file:/Users/jafinger/clj/clj-ns-browser/resource/abc%c3%add/foo.txt>
user=> (slurp (io/resource "abcíd/foo.txt"))
"The quick brown fox jumped over the lázy dög!\n"

The attached patch just binds *read-eval* to true inside source-fn.

Note: Allowing this implies that you trust the data on your classpath. If there are reasons somebody might not, we should reject this patch and people will have to be explicit when calling source.

Ugh, that's a fair point when it comes to sandboxing. I'll check with the owners of clojurebot and lazybot.

I haven't come up with any scenarios where this is problematic, and I haven't heard anything back from the bot owners. As for sandboxing, clojure.repl can easily be excluded.

version 1.5.0-RC17

I cross-posted this question to SO: http://stackoverflow.com/questions/15207596

calling RT.init() before Compiler.load() solves the problem

Yegor, do you consider it OK to close this ticket as not being a problem, or at least one with a reasonable workaround?

Yes, of course. Let's close it.

Ticket submitter agrees that this is not an issue, or that there is a reasonable workaround.

This issue came up again on the Clojure group. https://groups.google.com/forum/?hl=en_US&fromgroups=#!topic/clojure/2xdLNMb9yyQ

I did some testing, and the issue did not exist in Clojure 1.5.0-RC3 and before, and it has existed since 1.5.0-RC4. There was only one commit between those two points:

https://github.com/clojure/clojure/commit/9b80a552fdabeabdd93951a625b55ae49c2f8d83

Maybe this new behavior is an intended consequence of that change. I don't know. In any case, it seems like perhaps the "No need to call RT.init() anymore" message might be outdated?

Reopening since it came up again, and there is some more info known about the issue. I'll let someone who knows more about the issue decide whether to close it.

Doing this RT.load("clojure/core"); at the top works avoids the message from RT.init()

Windows

Proposed patch for issue
Process exceptions in macroexpand1 and wraps them in CompilerException with source,line,column information.

Also patch adds quotes around invalid symbol name in error message to make them more distinguishable from rest of message.

Patch 0001-CLJ-1169-proposed-patch.patch dated Feb 22 2013 causes several tests to fail. Run "./antsetup.sh" then "ant" to see which ones (or "mvn package").

Fix for failed unit-tests

Found this on ubuntu, lein 2, Clojure 1.5 snapshot, but it's obvious from inspection

Notes from the dev mailing list:

The "sources" JAR is generated by another Maven plugin, configured here:
https://github.com/clojure/clojure/blob/clojure-1.5.0-RC15/pom.xml#L169-L181

The simplest solution might be to just exclude the file from the sources jar. It looks like maven-source-plugin has an excludes option which would do the trick:

http://maven.apache.org/plugins/maven-source-plugin/jar-mojo.html#excludes

any

I kinda just realized it affects all versions since and including 1.2, because it appears that its implementation was the same since then.

If it's not meant to return the result of the delete, maybe it should specifically return nil and/or the doc say something?

As noted in a thread on the Clojure ML, you can pass a known value in the second argument position to detect a delete that failed:

(clojure.java.io/delete-file some-file :not-deleted)

This returns true on success and :not-deleted on failure.

However the docstring could be better worded to make that intention clear. Perhaps:

Delete file f. Return true if it succeeds. If silently is nil or false, raise an exception if it fails, else return the value of silently.
This allows you to detect whether the delete succeeded without catching an exception by passing a non-true truthy value as the second argument.

I think the most obvious solution would be to constrain the size of the cache. Adding an item to the cache is already not the fastest path, so a bit more work could be done to prevent the cache from growing indefinitely large.

That does raise the question of what criteria to use. Keep the first n entries? Keep the n most recently used (which would require bookkeeping in the fast cache-hit path)? Keep the n most recently added?

Note that this works as you might have hoped:

(-> [1 2 3] ((fn [args] (apply + args))))

because it expands into:

((fn [args] (apply + args)) [1 2 3])

Your suggestion that -> check for (fn ...) before treating it as a sequence and injecting the argument leaves open the question: Why only (fn ...) should be treated specially? Why not (let ...), (for ...), (doseq ...), etc? And if you go that far, how do you decide what should be allowed and what not?

I agree with Andy, that it's not realistic suggestion to check for fn,let,etc. Perhaps a doc fix would help here but I'm not sure if we just want to call out (fn ...). I'd recommend closing this unless Stephen speaks up.

I'm happy with Andy's synopsis of the problem, and it's reasonable not to change the behaviour of the threading macro specifically for (fn..).

However, this is a mistake that I'm sure many others make/have made and it's hard to diagnose what is going wrong without dumping interpreted form – hardly a reasonable expectation for a novice user.

Before closing this issue, I'd like to see improved failure reporting, such as causing the threading macro to throw a compile error or warning if passed a raw (unwrapped) function declaration (are there legitimate use cases this would affect?).

Dos not matter.

Using :pre and :post, IMO, isn't a good idea. Handling assertions is a two part game. The mechanism needs to account for both detection and reaction, and the latter is missing.

This isn't a perfect work-around, as it's a little verbose, but using Dire might work better than using extend. In addition, you get the "reaction" functionality that's missing from :pre and :post

Example for protocol preconditions: https://gist.github.com/4471276

@Michael I read your gist and the README of Dire. I think the supervision concept of Erlang has it's places but I don't like it for pre- and post-conditions. For me, such conditions have two proposes:

1. they should document the code and
2. they should fail fast to detect failures early.

To support my first point, your pre- and post-conditions are just lexical too far away from the actual function definition. For the second point: I think in the case of violations the program should just crash. One could maybe wrap some part of the program with one of your exception supervisors handling an AssertionError. But I don't think that handling pre- and post-condition violations for individual functions is a good thing.

@Alexander Indeed, your points are correct. Dire is meant to be exactly what you described. Lexically removed from application logic, and opportunity to recover from crashing. That was my best shot at aiding your needs quickly, anyway.

clojure 1.5 beta2, Mac OS X 10.8.2, java version "1.6.0_37"

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch allows a data-reader to return nil instead of throwing. Does sanity check that possible tag or record isJavaIdentifierStart(). Gives better error message for special characters that might actually be dispatch macros (rather than assuming it's a tagged literal).

clj-1138-data-reader-return-nil-for-no-op.patch allows a data-reader returning nil to be treated as a no-op by the reader (like #_). nil is not normally a useful value (actually it causes an exception in Clojure 1.4 through 1.5 beta2) for a data-reader to return. With this patch, one could get something like a conditional feature reader using data-readers.

clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch is the first patch to consider. It merely allows nil as a value from a data-reader and returns nil as the final value. I think it does what was originally intended for dispatch macros, and gives a better error message in many cases (mostly typos).

The second patch, clj-1138-data-reader-return-nil-for-no-op.patch, depends on the other being applied first. It takes an extra step to treat a nil value returned from a data-reader as a no-op for the reader (like #_).

It turns out that you can work around the original problem by having your data-reader return '(quote nil) instead of plain nil. That expression conveniently evaluates to nil so you can get a nil if necessary. This also works after applying the patches so there's still a way to return nil if you really want it.

(binding [*data-readers* {'x/nil (constantly '(quote nil))}] (read-string "#x/nil 42"))
;=> (quote nil)

Patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch dated Dec 22 2012 still applies cleanly to latest master if you use the following command:

% git am --keep-cr -s --ignore-whitespace < clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch

Without the --ignore-whitespace option, the patch fails only because some whitespace was changed in Clojure master recently.

OK, now with latest master (1.5.0-RC15 at this time), patch clj-1138-allow-data-reader-to-return-nil-instead-of-throwing.patch no longer applies cleanly, not even using --ignore-whitespace in the 'git am' command given above. Steve, if you could see what needs to be updated, that would be great. Using the patch command as suggested in the "Updating stale patches" section of http://dev.clojure.org/display/design/JIRA+workflow wasn't enough, so it should probably be carefully examined by hand to see what needs updating.

I removed my patches. Things have changes recently with the LispReader and new EdnReader.

Patch attached.

It may be easier to read the changes the patch does from within JIRA instead from the commit message, so I've added it here:

This solves two issues as specified by #CLJ-1134. Issue #1 is solved by doing a
relative jump forward within absolute-reposition in cl_format.clj, line 114 by
switching (- (:pos navigator) position) with (- position (:pos navigator)).

Issue #2 is handled by changing the default n-parameter to * depending on
whether the @-prefix is placed or not. If it is placed, then n defaults to
0, otherwise it defaults to 1.

In addition, new tests have been appended to test_cl_format.clj to ensure the
correctness of this patch. The tests have been tested on the Common Lisp
implementation GNU CLISP 2.49, which presumably handle the ~n@*
correctly. This patch and GNU CLISP returns the same output for each format
call, sans case for printed symbols; Common Lisp has case-insensitive symbols,
whereas Clojure has not.

Archlinux x86_64, Windows 7 x86_64

Shouldn't have set major priority; but I cannot edit issue again

Reduced priority to minor, since ticket creator could not do so themselves.

Apache Tomcat/6.0.24 JVM/1.6.0_26-b03 Linux 2.6.32-279.el6.x86_64

Clojure 1.4.0, Ring 1.1.6, Compojure 1.1.3, Lein-Ring Plugin 0.7.5 (for lein ring uberwar)

While it's reasonable to want this for your case, having long path names in a stacktrace could be inconvenient for others. I'd recommend posting your desired change on the dev list - https://groups.google.com/forum/?fromgroups#!forum/clojure-dev . If they're ok with it, then I'd recommend submitting a patch.

It looks like it's first trying to resolve a field by name, since field access with / is legal. For example:

user=> (Integer/parseInt)
CompilerException java.lang.NoSuchFieldException: parseInt, compiling:(NO_SOURCE_PATH:1)

user=> (Integer/MAX_VALUE)
2147483647

Would trying to resolve a method before a field fix this?

Tests pass.

Edward, your patch replaces the expression (or m1 {}) with m1. It was changed from m1 to (or m1 {}) in a commit on Oct 16, 2008 with descriptive text "improved nil handling in merge, merge-with", so I am pretty sure it would be best to leave it as (or m1 {}). I believe the intent is to allow all but one of the map arguments to merge-with be nil, and everything will still work.

The rest of the patch for avoiding one merge call seems sound to me.

Your change would be even better at preserving any metadata on the first non-nil map in the list, if instead of calling with the first map, it called it with the first non-nil item of the list, and then the rest of the list after that.

I figured out that `reduce1` did pass a head of the list for me. (https://github.com/clojure/clojure/blob/master/src/clj/clojure/core.clj#L887)
But case with first nil argument is still valid. Correct me, please, if i'm wrong.

I'm not sure about `(or m1 {})`. I don't see any problems which can happen. Probably behaviour of functions which are used internally was changed since 2008.

(contains? nil :a) ;=> false
(assoc nil :a 1) ;=> {:a 1}
(get nil :a) ;=> nil

I could write some tests for that.

(Fixed formatting)

x = {0: 1}
for item in stuff:
    x[item] = item * x.get(item - 1, 0)

(last (for-as [x {0 1}]
        [item stuff]
  (assoc x item (* item (get x (- item 1) 0)))))

(defmacro reduce-for [[res init] for-seq-exprs body-expr]
  `(reduce #(%2 %1) ~init
    (for ~for-seq-exprs
      (fn [~res]
        ~body-expr))))

(Someone please fix the formatting in above and delete this comment.)

Please change this to link to the definitive prose, so we don't have to maintain that in two places.

Feel free to correct the wording, I used something simple.

Stu, he linked to clojure.org as you requested so I'm moving this along.

I just realized that my patch also implements CLJ-1086.

Would be nice if tests also demonstrated that metadata is preserved correctly.

New patch in response to stuarthalloway feedback.

This patch also prevents an infinite loop in the macroexpander when fed the following expression:

(->> a b (->> c d))

Edit: Far simpler example.

The attached patch implements ex-message and ex-cause to work on arbitrary Throwables.

N.B. The primary use case I have for this, in case it matters, is interrupting the evaluation of a 'map' expression in which the mapped fn is slow to evaluate (throwing an InterruptedException), because I am not interested in its result any more. Then later I re-evaluate it because I am interested in the result after all, however with above bug the lazy sequence terminates instead of recommencing where it left off.

(UPDATE: This use case is similar to the kind of ersatz continuations that Jetty does (RetryRequest runtime exception) or even Clojure itself when barging STM transactions with the RetryEx exception.)

Related to CLJ-457 according to Christophe. His patch fixes this,too.

Sorry Christophe's patch doesn't work for me here. It avoids evaluating the LazySeq a second time by prematurely throwing an exception. However a LazySeq might evaluate properly the second time around b/c the situation causing the exception was transient. As per comment above an evaluation might get interrupted, throwing InterruptedException the first time around but not the second time.

Also the observation with the closure and macro need explanation IMHO.

further insight: 'delay' exhibits the same behavior and is a more simple case to examine. the macro suspicion is a red herring: as demoed below it is actually the closed over variable magically turns to nil, the when-let macro simply turned that into a nil for the whole expression.

(def delayed
  (let [a true]
    (delay
      (print "a=" a)
      (throw (Exception.)))))

(try
  (print "delayed 1:")
  (force delayed)
  (catch Exception ex (println ex)))

(try
  (print "delayed 2:")
  (force delayed)
  (catch Exception ex (println ex)))

prints:

delayed 1:a= true#<Exception java.lang.Exception>
delayed 2:a= nil#<Exception java.lang.Exception>

The above leads to dodgy outcomes such as: The following expression leads to an Exception on 1st evaluation and to "w00t!" on subsequent ones.

(def delayed
  (let [a true]
    (delay
      (if a
        (throw (Exception.))
        "w00t!"))))

Try it like this:

(try
  (print "delayed 1:" )
  (println (force delayed))
  (catch Exception ex (println ex)))

(try
  (print "delayed 2:")
  (println (force delayed))
  (catch Exception ex (println ex)))

Results in:
delayed 1:#<Exception java.lang.Exception>
delayed 2:w00t!

This code shows that the problem is tied to the :once flag as suspected.

(def test-once
  (let [a true]
    (^{:once true} fn* foo []
	    (println "a=" a)
	    (throw (Exception.)))))

Invoking the fn twice will show 'a' turning from 'true' to 'nil', try it like this:

(try
  (print "test-once 1:")
  (test-once)
  (catch Exception ex (println ex)))

(try
  (print "test-once 2:")
  (test-once)
  (catch Exception ex (println ex)))

Results in:
test-once 1:a= true
#<Exception java.lang.Exception>
test-once 2:a= nil
#<Exception java.lang.Exception>

That doesn't happen when the ^{:once true} is removed. Now one could argue that above fn is invoked twice, which is exactly what one is not supposed to do when decorated with the :once flag, but I argue that an unsuccessful call doesn't count as invocation towards the :once flag. The delay and lazy-seq macros agree with me there as the resulting objects are not considered realized (as per realized? function) if the evaluation of the body throws an exception, and realization/evaluation of the body is therefore repeated on re-evaluation of the delay/lazy-seq.

Try this using

(realized? delayed)

The :once flag affects this part of the compiler only. Some field is set to nil there in the course of a function invocation, for the good reason of letting the garbage compiler collect objects, however this should to be done after the function successfully completes only. Can this be changed?

A workaround for the case of the 'map' function as described in the 1st comment, works as this: The original map function, if you take out the cases for several colls, the performance enhancements for chunked seqs and forcing the coll argument to a seq, looks like this:

(defn map [f s]
  (lazy-seq
    (cons (f (first s)) (map f (rest s)))))

In my workaround I evaluate f twice:

(defn map [f s]
  (lazy-seq
    (f (first s))
    (cons (f (first s)) (map f (rest s)))))

Because the downstream functions that are slow to evaluate are all of the deref kind that cache their result (more lazy-seqs, delays, futures, promises), the InterruptedException can only happen during the 1st evaluation, while the tail call optimization that sets closed-over variables to nil (it pays to read this here: http://clojure.org/lazy) only happens on the second call. The first still creates an fn that captures the head of the sequence 's', however this is not being held onto as it is not returned.

I use this special version of map (and other, similarly rewritten functions based on lazy-seq such as iterate) when I want interruptible, restartable seq evaluations.

OS X, 10.8

That would be a potentially breaking change for some people's code that uses partition. partition-all behaves as you wish.

Also, your concern with the documentation is for when there are padding elements specified as an argument, but your examples don't specify any padding elements.

I agree, but I think the docs should then explicitly state: "if no padding is given, not all input elements may be returned in the output partitions" or something to that line.

More precise documentation of current behavior is always welcome in my opinion.

I've uploaded a patch that calls out when and how partition drops tail elements:
"If a pad collection is not supplied, any tail elements that remain from dividing the input collection length by n will not be included in a partition."

This patch implements the suggested feature.

The new system property is called clojure.core.loading-verbosely in analogy to the existing clojure.compile.warn-on-reflection.

maybe clojure should follow clojurescript's footsteps and move empty out of IPersistentCollection and create an
interface IEmptyableCollection extends IPersistentCollection { IEmptyableCollection empty(); }

Can whoever claims this please consider walk's behavior in the face of all different collection types? I think it also fails with Java collections.

Also, the collection partitioning code in clojure.data may be of use.

Mac OS, Java 6

The with-redefs doc (v1.4.0) says "These temporary changes will be visible in all threads." That sounds non-thread-safe to me.

In general, changes to var root bindings are not thread safe.

As I understand it, with-redefs is mainly used in test suites to mock out vars. It was introduced when vars became static by default and a lot of testsuites had been using binding for mocking. Maybe the docstring should be amended with something along the lines of: When using this you have to ensure that only a single thread is interacting with redef'd vars.

Behavior find as is, doc string change would be fine.

Patch clj-1104-doc-unsafety-of-concurrent-with-redefs-v1.txt dated Nov 25 2012 updates doc string of with-redefs to make it clear that concurrent use is unsafe.

clj-1103-make-conj-assoc-dissoc-handle-args-similarly-v1.txt dated Nov 4 2012 makes conj conj! assoc assoc! dissoc dissoc! handle args similarly to each other.

I too ran into this and started an additional discussion here: https://groups.google.com/d/topic/clojure-dev/wL5hllfhw4M/discussion

In particular, I don't buy the argument that (into coll xs) is sufficient, since into implies conj and there isn't an terse and idiomatic way to write (into map (parition 2 keyvals))

So +1 from me

clj-1102-improve-empty-stack-trace-handling-v1.txt dated Nov 4 2012 improves the handling of .getStackTrace returning a length 0 array in two places. I checked all other places .getStackTrace was called and they seem to already handle this case gracefully.

Vetting.

Add *default-data-reader-fn* to the special bindings in main.clj so that it is set!-able in the REPL

This is a one-liner that makes *default-data-reader-fn* convenient to use in the REPL, similar to how *data-readers* works. I'd like to have this fix in Clojure 1.5.

work-around for REPL:

(alter-var-root #'clojure.core/*default-data-reader-fn* (constantly my-default-reader))

Wouldn't it be better to make it read "Don't know how to create ISeq from: 2 (java.lang.Long)"? How many beginners will figure
out ex-data exists and how to use it?

Hi Michael,

ex-info messages should not, in general, pr-str things into their bodies. This raises the question of print-length and print-level in a place where the user doesn't have good control, while the whole point of ex-info is to be in the data business, not the string business. Users can control printing from ex-data any way they like.

There are two possible ways to make beginners aware of ex-data: Tell them about it in one (or a few places) in docs, or in an infinite number of places saying "This would have been useful here, but we didn't use it because you might not know about it." I prefer the former.

That said, I think it would be great to increase the visibility of ex-info and ex-data early on in documentation for beginners, and to make sure that things like exception printing in logs are flexible enough not to lose the benefits of ex-info.

This is the thread where Max Penet suggested to have 0-arity versions of the two fns:

https://groups.google.com/forum/?fromgroups=#!topic/clojure/IRlN-4LH_U0

Probably all, but observed on Ubuntu 12.04, OpenJDK 6

Sorry, I typo'd the example. (defn total ...) should be (defn sum ...).

fixed typeo in example

Couldn't reproduce the exception, but the 2nd example did chew through about 4x the amount of memory. Vetting.

adding a patch. Since most of Clojure ends up running this code in one way or another, I'd assert that tests are included as part of the normal Clojure test process.

Patch simply calls Util.ret1(argx, argx=null) on all invoke calls.

And as a note, both examples in the original report now have extremely similar memory usages.

Sounds great, and the patch looks good too. Let me know if I need to do anything else.

clj-1087-diff-perf-enhance-patch-v1.txt dated Oct 15 2012 implements Tom's suggested performance enhancement, although not exactly in the way he suggested. It does calculate the union of the two key sequences.

Clojure and ClojureScript

Shantanu, you give your name on the contributing page, but I don't see it there. Is your CA in transit, perhaps?

@Andy I can see my name on the contributors page when I search for the text "Shantanu". My CA was submitted more than a year ago.

Right you are. I somehow accidentally got my browser to enable case matching, and was expecting it to search case insensitively. Sorry for the noise.

Just for the record, the patch provided at http://dev.clojure.org/jira/browse/CLJ-1121 addresses this issue.

Fix for this defect.

The throwArity now attempts to locate and call clojure.main/demunge. If it finds the function it invokes it and uses the returned string in the error. Otherwise it just throws the actual class name. This results in the following behaviour:

user=> (defn a->b [])
#'user/a->b
user=> (a->b 32)
ArityException Wrong number of args (1) passed to: user/a->b clojure.lang.AFn.throwArity (AFn.java:449)
user=>

1.7.0-08, OS X 10.8

Confirmed to be broken on master. Vetting. Not sure what it's going to take to fix this, however. If this is of intrest for you, you might want to help push it along by providing a patch.

There is no code or ticket for this yet, but I wanted to mention that I've begun working on an implementation of RRB-Tree (Relaxed Radix Balanced Tree) vectors for Clojure (see discussion at https://groups.google.com/forum/?fromgroups=#!topic/clojure-dev/xnbtzTVEK9A). Assuming it is no big deal to get reducers to work on such vectors, subvec could be implemented in O(log n) time in such a way that the result was the same concrete type of vector as you started with, and thus reducers would work on them, too.

It would mean O(log n) time for subvec instead of today's O(1), but this would likely fix other limitations that exist today with subvec's, e.g. CLJ-761.

I have a fix for this and a regression test in the tree below:

https://github.com/mikera/clojure/tree/winfix

Not sure how best to make this into a patch though - I also have the pprint fix in here (CLJ-1076)

Attached a patch that I created with:

git format-patch winfix --stdout HEAD~3..HEAD > clj-1082.patch

Does this do the trick? I had to use the HEAD~3..HEAD to just get the most recent 3 commits and exclude the pprint changes that I needed in order to build on Windows.

Patch for CLJ-1082, containing 3 commits

Mike, your patch clj-1082.patch applies cleanly to latest master for me, so looks like you found one way to do it.

Another would be as follows, and closer to the directions on the JIRA workflow page: http://dev.clojure.org/display/design/JIRA+workflow (but not identical). Note that these commands would work on Mac OS X or Linux. I'm not sure what the correct corresponding command would be on Windows for the "git am" step below, unless that just happens to work because Windows and/or git implement the input redirection with "<" somehow.

1. Check out a fresh repo
   $ git clone git://github.com/clojure/clojure.git
   $ cd clojure

1. Apply the patch for CLJ-1076 to the master branch. This step isn't on the JIRA workflow page.
   $ git am --keep-cr -s < clj-1076-fix-tests-on-windows-patch-v1.txt

1. Create a branch for yourself
   $ git checkout -b fix-clj-1082

1. After editing to make your changes, commit them to the current fix-clj-1082 branch
   $ git commit -a -m "fixed annoying bug, refs #42"

From there on down it is the same as the instructions on the JIRA workflow page. The "git format-patch master --stdout > file.patch" will create a patch for the changes you have made in the current branch fix-clj-1082 starting from the master branch, which has the CLJ-1076 fix because of the 'git am' command above.

Patch clj-1080-eliminate-many-reflection-warnings-patch-v1.txt dated Sep 30 2012 adds type hints to eliminate many uses of reflection in Clojure core code.

clj-1080-eliminate-many-reflection-warnings-patch-v2.txt dated Nov 14 2012 is identical to the previous patch (to be removed soon), except it applies cleanly to latest master.

clj-1080-eliminate-many-reflection-warnings-patch-v3.txt dated Feb 7 2013 is identical to the previous patch (to be removed soon), except it applies cleanly to latest master. One type hint in the patch was added due to a different change, and was no longer needed in the patch.

Please bring this up on clojure-dev. We'll be able to vet this ticket after that.

Should I respond to my original clojure-dev post about this (linked in the issue description above), or start a new one?

In a quickly hacked up performance test on Mac OS X 10.6.8 + Oracle/Apple JDK 1.6.0_35 which I can attach, I saw about a 9% to 10% slowdown for Colin's patch in printing large vectors.

I have a modified patch that only calls Thread/interrupted after every 20 items are printed, and with that version the slowdown versus the original code was about 3% to 4%.

Colin, would there be any down side to checking Thread/interrupted less often for your purposes?

To run performance test, edit attachment compile.sh to point at your clojure.jar, put file perftest-print.clj in the same directory, and run ./compile.sh It should work on Mac or Linux.

clj-1073-allow-thread-interrupt-in-print-sequential-patch.txt dated Sep 22 2012 is similar to Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks interrupted status every 20 (or maybe 21?) times through the loop in print-sequential. It is the one that is 3-4% slower than the current latest master Clojure code in my performance test mentioned above, versus Colin's patch which is about 9-10% slower on that test.

Is this primarily intended for dev-time use? I wouldn't want to lose performance for this if there is any way to implement it as a dev-time feature.

Andy: The only caveat I can think of with checking Thread/interrupted less often is in the case of deeply nested collections.

Stu: Dev-time use was the original reason for opening this, yes. But I can imagine it being needed for anytime a thread can be interrupted, whether that's by ctrl-c or other means.

My original thinking, performance-wise, was that once we're already doing IO, we're probably not too concerned with CPU-bound checks like this, so I didn't bother with it. I guess with an SSD that's not as likely to be true.

Locally (w/ my SSD), I'm seeing that Andy's benchmark of printing a million numbers is about a second slower than the baseline with my original patch (12.08s -> 13.10s), and about the same with Andy's patch (12.08s -> 11.75s). Decreasing to a thousand numbers doesn't really show any difference (every version completes in ~1.3s). Bumping to 2 million adds 2 seconds above the baseline with my patch, and Andy's is again just a bit faster than the baseline (somehow). Bumping to 5 million runs me out of heap space

clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 is the same idea as Colin's patch 0001-Allow-thread-interruption-in-print-sequential.patch dated Sep 21 2012, except it only checks (Thread/interrupted) if a new var print-interruptibly is true. Its default value is false.

Performance results of the perftest-print.clj program, as driven by the test.sh script, for Clojure 1.5-beta1 and with two different patches. All run times are elapsed, in seconds, and sorted in increasing order for easier comparison.

Executive summary: Performance results when print-interruptibly is left at default false value are pretty much same as today. With print-interruptibly bound to true, performance results are slower, as expected, and about the same as with Colin's patch.

Original 1.5-beta1 unchanged:
13.75 13.80 13.83 13.87 13.93

With this new print-interruptibly patch, with print-interruptibly
at default false value:
13.86 13.91 14.01 14.08 14.14

With this new print-interruptibly patch, with print-interruptibly
bound to true while printing (so a slightly modified version of
perftest-print.clj that does (binding [*print-interruptibly* true]
...) around the heart of the code):
15.29 15.44 15.45 15.62 15.63

With patch 0001-Allow-thread-interruption-in-print-sequential.patch
applied:
15.38 15.46 15.48 15.49 15.50

Andy's latest patch looks good & makes it easy for the REPL and other interruptible scenarios to opt into the "safe" behavior. I would personally have preferred to have the "safe" behavior on by default, but can understand the performance concerns, and this gets me what I really wanted.

Vetting as it sounds like the performance issues are taken care of.

clj-1073-add-print-interruptibly-patch-v2.txt dated Feb 12 2013 is identical to clj-1073-add-print-interruptibly-patch-v1.txt dated Nov 8 2012 (soon to be removed) except that it applies cleanly to latest master.