<< Back to previous view

[CLJ-1638] Regression - PersistentVector.create(List) was removed in 1.7.0-alpha5 Created: 12/Jan/15  Updated: 16/Jan/15

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, regression


Attachments: Text File clj-1638.patch    
Patch: Code
Approval: Screened


For CLJ-1546, PersistentVector.create(List) was replaced with PersistentVector.create(ArrayList). At least one library (flatland) was calling this method directly and was broken by the change.

Approach: Replace the prior implementation.

Patch: clj-1638.patch

Screened by:

[CLJ-1637] vec fails on MapEntry Created: 11/Jan/15  Updated: 16/Jan/15

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

Type: Defect Priority: Critical
Reporter: Alex Miller Assignee: Alex Miller
Resolution: Unresolved Votes: 0
Labels: regression


Attachments: Text File clj-1637-jdevuyst.patch     Text File clj-1637.patch     Text File clj-1637-with-test.patch    
Patch: Code
Approval: Screened


After CLJ-1546:

(vec (first {1 2}))

Cause: (if (vector? coll) (with-meta coll nil) ...) checks that something is IPersistentVector, then sends it to something that takes IObj, so anything that is one but not both throws an error. In Clojure itself, this is the set of classes extending from AMapEntry.


1. Make AMapEntry implement IObj - this fixes everything in Clojure and keeps the vec code as is but still leaves open this gap for any external implementation of IPersistentVector.
2. Check for this case explicitly in vec. (if (vector? coll) (if (instance? clojure.lang.IObj coll) (with-meta coll nil) (clojure.lang.LazilyPersistentVector/create coll)) ...). Perf testing shows no significant difference in performance with the change.
3. Pull the special check for vector? in vec.
4. Check for this case explicitly in vec and return the same instance if it's not an IObj. See clj-1637-jdevuyst.patch.

Approach: patch takes approach #2

Patch: clj-1637-with-test.patch

Screened by: Stu (also added test)

Comment by Nicola Mometto [ 11/Jan/15 8:19 AM ]

The correct fix is to probably make MapEntry an IObj

Comment by Nicola Mometto [ 11/Jan/15 8:21 AM ]

Actually, making AMapEntry an IObj rather than MapEntry would fix the issue for sorted-map kv-pairs too.

user=> (vec (first (sorted-map 1 1)))
ClassCastException clojure.lang.PersistentTreeMap$BlackVal cannot be cast to clojure.lang.IObj  clojure.core/with-meta--4121 (core.clj:216)
Comment by Alex Miller [ 11/Jan/15 8:35 AM ]

There are potentially a couple ways to fix this. I'll look at it next week.

Comment by Jonas De Vuyst [ 14/Jan/15 6:14 AM ]

Slightly modified patch. In the case where coll is a vector but not an IObj, simply return coll.

Comment by Jonas De Vuyst [ 14/Jan/15 7:17 AM ]

If desired I could also update the patch to fix an analogous—albeit somewhat theoretic—bug in `set`.

It does make me wonder if perhaps a `without-meta` function should be added to `clojure.core`.

I think making AMapEntry an IObj might make sense even after applying the above patches. In `AMapEntry`, perhaps `withMeta(m)` could be implemented as `asVector().withMeta(m)`. This, however, would require changing `asVector()` to return some `IObj ∩ IPersistentVector` type (e.g. `PersistentVector`). This would be straightforward to do, but requires deciding if this change in signature may be propagated to the static methods of `LazilyPersistentVector`.

Comment by Alex Miller [ 14/Jan/15 9:28 AM ]

This is a point of some debate, but the intention with the change in the implementation is to retain current behavior, which always gives you a new vector instance. It's not clear to me that there is any point in attaching meta to map entries (which also does not solve the problem for external IPersistentVector, non-IObj instances outside Clojure).

In any case, I'm going to update the description a bit to add this as an alternative.

[CLJ-1604] AOT'ed code that defs a var with clojure.core symbol name causes IllegalStateException Created: 25/Nov/14  Updated: 16/Jan/15

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

Type: Defect Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: aot, compiler

Attachments: Text File 0001-fix-AOT-bug-preventing-overriding-of-clojure.core-fu.patch     Text File 0001-fix-AOT-bug-preventing-overriding-of-clojure.core-fu-v2.patch     File 1604-context.diff    
Patch: Code
Approval: Screened


AOT'ed code that defs a var that is also a symbol in clojure.core results in an exception at runtime. This problem can be avoided with (:refer-clojure :exclude ...) but this requires a library author to update and release a new version. AOT'ed applications must then wait for all transitive dependencies to update before they can update to a new Clojure version. For some users, this problem prevents them from trying or adopting new releases.

For example, the contrib library data.int-map defines an update function. clojure.core will also have a new update function as of 1.7.0. If this library is AOT'ed, then users of the clojure.data.int-map/update function will see the exception below. This situation can commonly occur when an application uses lein uberjar to compile all of the project+libs. In this case, applications or libraries that use data.int-map (either directly or indirectly) are affected.

java.lang.IllegalStateException: Attempting to call unbound fn: #'clojure.data.int-map/update
 at clojure.lang.Var$Unbound.throwArity (Var.java:43)
    clojure.lang.AFn.invoke (AFn.java:40)
    compiler_update_not_referenced_bug.core$foo.invoke (core.clj:5)

Reproduce with this sample project: https://github.com/yeller/compiler_update_not_referenced_bug

Cause: When AOT compiling a namespace, the def forms are hoisted into the ns__init class (in the example here, clojure.data.int_map__init). The static initializer in this class creates each var in the ns via a call to RT.var(ns, name). For data.int-map the static initializer will properly create the var for clojure.data.int-map/update. But when the ns is loaded (via the clojure.data.int_map.load() method), (refer-clojure) will be called, which will remap clojure.data.int-map/update to point to clojure.core/update.

This problem does not affect non-AOT loading (which doesn't use the ns__init class) and does not affect collisions from any other namespace. Only collisions from clojure.core create this possibility.

Proposed: The proposed patch explicitly refers the Var during ns__init.load() (after Clojure symbols are referred) rather than implicitly during ns__init static {}.

This change in behavior only happens during AOT in the specific case where a core symbol is being shadowed. In that case, clojure.core has already been loaded and v (the looked up var) will have ns=clojure.core. The currentNS will be (for example) data.int-map. If that's the case, and the sym has no ns, then the new logic will be emitted.

In the case of clojure.core itself, NO new bytecode is emitted. From tests on several projects, only shadowed vars during AOT get this additional bytecode.

Patch: 0001-fix-AOT-bug-preventing-overriding-of-clojure.core-fu-v2.patch

Screened by: Alex Miller

Comment by Andy Fingerhut [ 25/Nov/14 11:28 PM ]

When I try latest Clojure master plus patch CLJ-1604-only-core.patch with the small test project created by Tom Crayford to demonstrate this issue: https://github.com/yeller/compiler_update_not_referenced_bug

In that project, I get the same exception thrown when attempting 'lein do clean, uberjar, test' using this patch, as without it. It is because int-map/update in namespace compiler-update-not-referenced-bug.core is an unbound var.

Comment by Nicola Mometto [ 26/Nov/14 4:25 AM ]

Andy, you're right. For some reason I attached the wrong patch to the ticket, this is the correct one

Comment by Nicola Mometto [ 26/Nov/14 5:21 AM ]

I wasn't able to write a test for this, so here's a repl session using the clojure jar demonstrating this issue:

[˷/test]> ls
classes  clojure.jar  test.clj
[˷/test]> cat test.clj
(in-ns 'test)
(clojure.core/refer 'clojure.core)
(def foo "bar")
(def update "foo")
[˷/test]> java -cp classes:clojure.jar:. clojure.main
Clojure 1.7.0-master-SNAPSHOT
user=> (binding [*compile-files* true] (load "test"))
WARNING: update already refers to: #'clojure.core/update in namespace: test, being replaced by: #'test/update
user=> test/foo
user=> test/update
[˷/test]> java -cp classes:clojure.jar:. clojure.main
Clojure 1.7.0-master-SNAPSHOT
user=> (load "test")
user=> test/foo
user=> test/update
CompilerException java.lang.RuntimeException: No such var: test/update, compiling: (NO_SOURCE_PATH:0:0)
Comment by Andy Fingerhut [ 26/Nov/14 10:39 AM ]

Thanks. I have not tried to assess the details of the change, other than to say that patch 0001-fix-AOT-bug-preventing-overriding-of-clojure.core-fu.patch dated 26 Nov 2014, when applied to latest Clojure master as of today, enables both 'lein do clean, test' and 'lein do clean, uberjar, test' to work as expected with Tom Crayford's test project, linked above, whereas 'lein do clean, uberjar, test' fails without this patch, due to a var being unbound that should have a value.

Comment by Andy Fingerhut [ 27/Nov/14 10:53 AM ]

Copying a comment here from CLJ-1591, since it is more appropriate here. It is responding to Tom Crayford's posting of his example project to demonstrate the issue: https://github.com/yeller/compiler_update_not_referenced_bug

Tom, looked at your project. Thanks for that. It appears not to have anything like (def inc inc) in it. It throws exception during test step of 'lein do clean, uberjar, test' consistently for me, too, but compiles with only warnings and passes tests with 'lein do clean, test'. I have more test results showing in which Clojure versions these results change. To summarize, the changes to Clojure that appear to make the biggest difference in the results are below (these should be added to the new ticket you create – you are welcome to do so):

Clojure 1.6.0, 1.7.0-alpha1, and later changes up through the commit with description "CLJ-1378: Allows FnExpr to override its reported class with a type hint": No errors or warnings for either lein command above.

Next commit with description "Add clojure.core/update, like update-in but takes a single key" that adds clojure.core/update: 'lein do clean, test' is fine, but 'lein do clean, uberjar' throws exception during compilation, probably due to CLJ-1241.

Next commit with description "fix CLJ-1241": 'lein do clean, test' and 'lein do clean, uberjar' give warnings about clojure.core/update, but no errors or exceptions. 'lein do clean, uberjar, test' throws exception during test step that is same as the one I see with Clojure 1.7.0-alpha4. Debug prints of values of clojure.core/update and int-map/update (in data.int-map and in Tom's namespace compiler-update-not-referenced-bug.core) show things look fine when printed inside data.int-map, and in Tom's namespace when not doing the uberjar, but when doing the uberjar, test, int-map/update is unbound in Tom's namespace.

In case it makes a difference, my testing was done with Mac OS X 10.9.5, Leiningen 2.5.0 on Java 1.7.0_45 Java HotSpot(TM) 64-Bit Server VM

Comment by Nicola Mometto [ 02/Dec/14 9:04 AM ]

The updated patch only emits the interning bytecode when necessary, avoiding the emission when a clojure.core var with the same name exists but is not mapped to the current namespace

Comment by Alex Miller [ 09/Jan/15 9:27 AM ]

Attached 1604-context.diff for purely informational purposes - same diff just more context in it for easier reading.

Comment by Tom Crayford [ 10/Jan/15 4:52 PM ]

Thought I'd add a minor note in here to say I tried testing this patch out on my app (which is where I discovered this AOT bug), and the bug doesn't turn up with this patch applied to clojure (tested by applying 0001-fix-AOT-bug-preventing-overriding-of-clojure.core-fu-v2.patch to 1.7-alpha5)

[CLJ-1568] Incorrect error locations reported in the stacktrace Created: 19/Oct/14  Updated: 14/Jan/15

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

Type: Defect Priority: Major
Reporter: Bozhidar Batsov Assignee: Unassigned
Resolution: Unresolved Votes: 19
Labels: errormsgs, ft

Attachments: Text File 0001-CLJ-1568-fix-incorrect-error-locations.patch     Text File 0001-CLJ-1568-fix-incorrect-error-locations-v2.patch     Text File clj-1568.patch    
Patch: Code and Test
Approval: Screened


The following code produces an incorrect stacktrace:

(ns clojure-demo.core)

(defn foo
  "I don't do a whole lot."
  (println x "Hello, World!"))

(/ 1 0)
Exception in thread "main" java.lang.ArithmeticException: Divide by zero, compiling:(clojure_demo/core.clj:6:31)

The problem is actually on the 8th line. As a matter of fact - there's nothing at location 6:31.
This is a pretty serious problem as many tools parse stacktraces for error locations.
Here's a related discussion in cider's issue tracker.

Patch: clj-1568.patch
Screened by: Alex Miller

Comment by Alex Miller [ 19/Oct/14 1:39 PM ]

Maybe a dupe of CLJ-1561 ?

Comment by Andy Fingerhut [ 19/Oct/14 4:16 PM ]

I tried out the example given in the description, with the latest Clojure master as of today plus the patch for CLJ-1561 called 0002-Mark-line-number-after-emitting-children.patch, dated Oct 10 2014.

The line:column number 6:31 is the same for that patched version as it is in the ticket description, which is for Clojure 1.6.0.

The issue of misleading line:column numbers is common between the two tickets, but at least the proposed improvement in CLJ-1561's patch is not effective for improving this issue.

Comment by Bozhidar Batsov [ 20/Oct/14 1:36 AM ]

I know that the issue list for 1.7 is pretty much finalised, but I think that this issue and and CLJ-1561 should be fixed as soon as possible.
Correct error reporting is extremely important IMO.

Comment by Nicola Mometto [ 20/Oct/14 8:28 AM ]

Attached a patch that fixes the issue by consuming all the whitespaces before retrieving line/column info for the next form.

Comment by Alex Miller [ 20/Oct/14 8:39 AM ]

Are there possible downsides to more eagerly consuming whitespace as done in the patch?

Comment by Nicola Mometto [ 20/Oct/14 8:44 AM ]

I can't think of any

Comment by Paul Stadig [ 22/Oct/14 2:59 PM ]

The defect on master does not have effect when using compile:

user=> (require 'clojure-demo.core)

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(clojure_demo/core.clj:6:31) 
user=> (load "/clojure_demo/core")

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(clojure_demo/core.clj:6:31) 
user=> (compile "clojure_demo/core")

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(core.clj:8:1) 

With the patch applied all the line numbers are the same in all cases:

user=> (require 'clojure-demo.core)

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(clojure_demo/core.clj:8:1) 
user=> (load "/clojure_demo/core")

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(clojure_demo/core.clj:8:1) 
user=> (compile "clojure_demo/core")

CompilerException java.lang.ArithmeticException: Divide by zero, compiling:(core.clj:8:1) 

Agreed that this seems to be orthogonal to CLJ-1561.

Comment by Bozhidar Batsov [ 10/Jan/15 3:08 AM ]

Seems we need to add tests before this can be merged - https://groups.google.com/forum/#!topic/clojure-dev/7pFhG8LMvGo

Comment by Daniel Solano Gómez [ 11/Jan/15 12:20 AM ]

Well, I've been looking into adding tests to this patch, and I've made some interesting discoveries. The additions to Compiler.load() seem to work just fine. However, I'm not seeing much coming out of the changes to Compiler.compile. You can see in Paul's comment above that the compile call actually reports the correct location. I created a test that throws a NullPointerException during compilation, and in the case of compile, it is never wrapped in a CompilerException. I'll attach my test patch that contains the example code I have been working with.

Comment by Daniel Solano Gómez [ 11/Jan/15 12:26 AM ]

Patch with tests for code paths in the fix. The tests for the Compiler.compile changes are not showing what I had expected.

Comment by Alex Miller [ 12/Jan/15 8:07 AM ]

Whenever people feel this is ready for screening, just switch the Approval from Incomplete to Vetted.

Comment by Nicola Mometto [ 12/Jan/15 8:39 AM ]

Updated patch removing the changes to Compiler.compile as they seem to be useless, by Daniel's tests

Comment by Daniel Solano Gómez [ 14/Jan/15 10:26 AM ]

I have cleaned up my test code a bit and put together a combined patch that includes both the fix and the tests.

Comment by Bozhidar Batsov [ 14/Jan/15 11:12 AM ]

Great! Looking forward to seeing this merged.

[CLJ-1561] Incorrect line numbers are emitted Created: 10/Oct/14  Updated: 15/Jan/15

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

Type: Defect Priority: Major
Reporter: Paul Stadig Assignee: Unassigned
Resolution: Unresolved Votes: 23
Labels: errormsgs

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


The Clojure JVM compiler marks the line number for a form before emitting the children for that form. Marking the line number before emitting children leads to incorrect line numbers when a runtime error occurs. For example, when

 (foo bar

is emitted the compiler will visit the line number for the expression, then emit the children expressions ('bar' and 'baz') which will mark their own line numbers, then come back and emit the invoke bytecode for 'foo', but since the last line number to be marked was that of 'baz', if 'foo' throws an exception the line number of 'baz' will be reported instead of the line number for the expression as a whole.

This same issue was being manifested with special forms and inlined functions, and was especially bad in the case of the threading macro '->', because it is usually spread across several lines, and the line number reported could end up being very different than the line actually causing an exception.

A demonstration of the incorrect line numbers (and how the fix affects line numbers) can be seen here https://github.com/pjstadig/clojure-line-numbers

Patch: clj-1561.patch

Screened by: Alex Miller

Comment by Jozef Wagner [ 10/Oct/14 1:57 PM ]

additions in your patch mixes tabs and spaces. Could you please update the patch so that your added lines indent only with tab characters? Not everyone has tab set at 4 spaces...

Comment by Paul Stadig [ 10/Oct/14 2:42 PM ]

There's already a mixture of just tabs, just spaces, and tabs & spaces in Compiler.java. I'm not sure what the "standard" is, but I've changed the patch to match the surrounding lines.

Comment by Paul Stadig [ 10/Oct/14 2:42 PM ]

Patch with whitespace changes.

Comment by Alex Miller [ 20/Oct/14 8:38 AM ]

These changes will affect the line number tables for a variety of Clojure constructs when compiled. It would be very helpful to me to have a set of examples that covered each case touched in the patch so that I could compile them and look at the bytecode vs the source. This would greatly accelerate the screening process.

Comment by Paul Stadig [ 20/Oct/14 2:29 PM ]

I have created a repo on github that has a sample file demonstrating the line number changes.


Hope that helps!

BTW, I'd be glad to do a skype call or hangout, if you have questions.

Comment by Alex Miller [ 20/Oct/14 2:34 PM ]

This is very helpful, thanks!!

Comment by Alex Miller [ 22/Oct/14 11:35 AM ]

In the hunk at 3191 in KeywordInvokeExpr, a call to visitLineNumber was added, but the prior call 4 lines earlier was not removed. Should it be?

Comment by Paul Stadig [ 22/Oct/14 12:05 PM ]

I left that in thinking that if something goes wrong with the getstatic instruction (null pointer exception? class cast exception?) it should report the line number of the KeywordInvokeExpr. It may be that there isn't a realistic possibility that anything could actually happen with that getstatic instruction, but that was the thought process.

My general rule of thumb was if an emit method emits any instructions before it calls the emit method on another expr, then it should mark its line number before and after the recursive emit call (assuming that the recursive emit call would mark its own line number). In cases where an emit method immediately calls another emit method, then I don't bother to mark a line number until afterwards.

Comment by Bozhidar Batsov [ 10/Jan/15 3:09 AM ]

Seems we need to add tests before this can be merged - https://groups.google.com/forum/#!topic/clojure-dev/7pFhG8LMvGo

Comment by Daniel Solano Gómez [ 11/Jan/15 8:42 PM ]

Here's an updated version of Paul's patch that applies cleanly to master.

I based my test code from his GitHub repository, but I have made a few changes:

  1. I have added test cases for when emitUnboxed or the reflecting branch of emit is called.
  2. The original keyword invocation test doesn't actually go through KeywordInvokeExpr. It is resolved by the change to InvokeExpr.emitArgsAndCall, so I have removed it.

Additionally, I have been unable to create test cases that verify that the changes for intrinsic predicates, KeywordInvokExprs, and for the changes inside of InvokeExpr.emit. These could be that they are redundant, but I am not sure.

Comment by Paul Stadig [ 12/Jan/15 5:51 AM ]

Thanks Daniel!

I applied the patches for my commit (as updated by Daniel) and Daniel's commit. The namespace docstring had shifted all the line numbers down by 4, and the tests were failing. I amended Daniel's commit with updated line numbers and combined both commits into a single patch file (clj-1561.patch).

I also reverted my commit and re-ran Daniel's tests to verify that they failed as expected.

I believe the combined patch file should include everything we need other than the difficult test cases that Daniel mentions in his comment.


Comment by Alex Miller [ 12/Jan/15 8:07 AM ]

Whenever people feel this is ready for screening, just switch the Approval from Incomplete to Vetted.

Comment by Paul Stadig [ 12/Jan/15 2:20 PM ]

I have added a test for KeywordInvokeExpr, and a new test to test the logic in InvokeExpr.emit.

I removed the changes I made to StaticMethodExpr.emitIntrinsicPredicate, since I was unable to find a way to create a test case. The issue is that we mark the lines of child expressions, then come back to the parent expression and try to invoke without marking its line, but in the case of intrinsic predicates there is no invocation. When we come back to the parent we directly emit some bytecodes. I don think the change I made in that case actually changed anything, but I removed it since there is no need for it.

Comment by Alex Miller [ 14/Jan/15 10:07 PM ]

Could we squash the patch into a single commit?

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

Also, in addition to squashing the patch, can we rename examples_clj_1561.clj to line_number_examples.clj or something w/o the ticket in it? I'm ready to mark as screened when those are done. (And if that can get done today, it's likely to get pushed forward tomorrow.)

Comment by Daniel Solano Gómez [ 15/Jan/15 9:24 AM ]

In case Paul doesn't get to it sooner, I can take care of it this afternoon.

Comment by Paul Stadig [ 15/Jan/15 10:40 AM ]

I'd like to maintain attribution. I could squash my test related commits into Daniel's commit, so we'd have two commits, my commit for the change and Daniel's for the tests. Would that suffice? If so, I can take care of it now.

Comment by Alex Miller [ 15/Jan/15 11:13 AM ]

Sure, that's fine.

Comment by Alex Miller [ 15/Jan/15 11:14 AM ]

note that one of the commits has a whitespace error (I think Daniel's) that got removed in a later commit. In the end patch, please make sure it doesn't warn about any whitespace errors when applied.

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

Paul, this patch still seems to have examples_clj_1561.clj in it - would prefer name that is meaningful vs jira issue.

Comment by Paul Stadig [ 15/Jan/15 1:44 PM ]

Yeah, sorry, I'm still working on it. Should be up shortly.

Comment by Paul Stadig [ 15/Jan/15 2:05 PM ]

I squashed the commits down to two: my commit for the change, and Daniel's for the tests. I renamed the "examples_clj_1561" file to "line_number_examples". I removed some trailing whitespace from Daniel's commit. There are still whitespace errors (on my machine) from my commit because tabs are used for indenting. I used tabs to indent because that is how the lines surrounding were formatted, and Jozef had already complained about the formatting. It also appears that these whitespace errors are dependent on your git config. I have:

whitespace = trailing-space,space-before-tab,tab-in-indent

But when I comment that out of my gitconfig I get no whitespace errors. I guess the bottom line is if we want to agree on a standard config for that I'd be glad to accomodate, but I don't think we want to reformat Compiler.java, so the best choice seems to be to just match the surrounding context, which is what I've done. If you want me to do anything further with that, then let me know.

Ball is back in your court, Alex.

[CLJ-1589] Cleanup internal-reduce implementation Created: 14/Nov/14  Updated: 15/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-cleanup-internal-reduce-impl.patch    
Patch: Code
Approval: Screened


Currently internal-reduce provides an implementation for ArraySeq and the ArraySeq_* prim classes.
Since those classes implement IReduce the current patch makes instances of those classes fallback on coll-reduce's IReduce impl (that simply invokes .reduce)

This change is desiderable because it removes unnecessary duplicated code, reducing the implementation surface and making it easier to follow reduce's code path. In addition to ArraySeq there will be (based on other tickets) more seq impls that also IReduce, so it would be good to re-route back through coll-reduce when we get combinations of potentially reducible sub-seqs.

Patch: 0001-cleanup-internal-reduce-impl.patch

  • This patch depends on the patch for CLJ-1590 since the current IReduce impl for those ArraySeq classes doesn't properly handle Reduced

Screened by: Alex Miller

Comment by Alex Miller [ 14/Nov/14 10:28 PM ]

I'm not sure whether this should be in 1.7 or not, but I'm adding it there so we can have a discussion on it regardless.

[CLJ-1515] Reify the result of range and add IReduceInit Created: 29/Aug/14  Updated: 23/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Timothy Baldridge Assignee: Alex Miller
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File clj-1515-10.patch     Text File clj-1515-2.patch     Text File clj-1515-3.patch     Text File clj-1515-4.patch     Text File clj-1515-5.patch     Text File clj-1515-6.patch     Text File clj-1515-7.patch     Text File clj-1515-8.patch     Text File clj-1515-9.patch     Text File CLJ-1515-deftype2.patch     Text File CLJ-1515-deftype3.patch     Text File CLJ-1515-deftype-nostructural-dup.patch     Text File CLJ-1515-deftype.patch     Text File clj-1515.patch     File patch.diff     File range-patch3.diff     File reified-range4.diff    
Patch: Code and Test
Approval: Screened


Currently range returns a lazy chunked seq. If the return value of range were reified into a type we could optimize common cases and add IReduce support.

Approach: this patch implements range as a deftype. The range implementation is specialized for primitive longs (LongRange, a common case), as well one with generic math (GenericRange).

The types implement IReduce,IReduceInit,Counted,Seqable,IHashEq. Sequential and Serializable are marker interfaces.

The existing range lazy-seq was renamed to range* (instead of range1 b/c range's arguments are numbers.) The deftypes defined later delegate their seq implementation to it. Still chunked. To achieve the most performance out of this delegation, it required a tiny tweak to doall/dorun, see the note below. range* handles only ascending or descending sequences.

The implementation was carefully tuned using criterium for benchmarking. Rather than directly using < and > as comparators, it performed far better to close over the range upper bound, e.g. #(< % end). < and > are inlining macros. The boxing/unboxing is minimized within the LongRange implementation.

The range constructor figures out type specialization, calls create-range, which filters out the wacky cases (empty seqs, the repeat case), and then creates the types if necessary. The deftypes handle only ascending or descending nonempty cases.

The special case of (range) is just handled with (iterate inc' 0) (which is further optimized for reduce in CLJ-1603). The original contract range being infinite was never handled correctly, as it used inc, not inc'. The GenericRange impl does not use autopromotion, and opts for preserving the implementation behavior. The only path for auto-promotion with this patch is through the 0-arity delegation to iterate.

Note The patch also includes a tweak to doall/dorun in order to handle types whose seq implementation is not inline, but in a separate object. As noted in the commit message, dorun currently calls (seq coll), then drops the result, then calls (next coll), resulting in a double allocation of the seq when the collection doesn't cache its seq.

timings done via criterium full benchmark on Oracle JDK

code 1.7.0-alpha5 Java impl clj-1515-10 CLJ-1515-deftype2
(count (range (* 1024 1024))) 61.30 ms 26.22 ns 12.67 ns
(reduce + (map inc (range (* 1024 1024)))) 56.21 ms 47.80 ms 46.87 ms
(reduce + (map inc (map inc (range (* 1024 1024))))) 77.98 ms 67.84 ms 66.91 ms
(count (keep odd? (range (* 1024 1024)))) 73.68 ms 57.33 ms 72.27 ms
(transduce (comp (map inc) (map inc)) + (range (* 1024 1024))) 50.56 ms 36.12 ms 28.46 ms
(reduce + 0 (range (* 2048 1024))) 81.86 ms 39.22 ms 26.44 ms
(doall (range 0 31)) 1.32 µs 2.89 µs 1.08 µs
(doall (range 0 32)) 1.34 µs 2.96 µs 1.12 µs
(doall (range 0 128)) 5.20 µs 11.92 µs 4.31 µs
(doall (range 0 512)) 21.24 µs 47.71 µs 17.91 µs
(doall (range 0 4096)) 167.86 µs 381.15 µs 141.48 µs
(into [] (map inc (range 31))) 2.00 µs 1.49 µs 1.87 µs
(into [] (map inc) (range 31)) 1.65 µs 813.78 ns 929.51 ns
(into [] (range 128)) 5.20 µs 2.27 µs 2.44 µs
(doall (range 1/2 1000 1/3)) 1.50 ms 1.69 ms 1.50 ms
(doall (range 0.5 1000 0.33)) 172.27 µs 364.35 µs 149.51 µs
(into [] (range 1/2 1000 1/3)) 1.52 ms 1.42 ms 1.43 ms
(into [] (range 0.5 1000 0.33)) 163.06 µs 99.44 µs 103.18 µs
(count (filter odd? (take (* 1024 1024) (range)))) 187.79 ms 194.07 ms 189.73 ms
(transduce (take (* 1024 1024)) + (range)) 53.06 ms 94.19 ms 89.34 ms

Performance notes:

doall cases see a substantial benefit.

The deftype has very competitive performance considering it is competing against the Java impl, which uses unchecked math incorrectly.

The final two cases of 0-arity (range) will get better pending the merge of the Iterate deftype (CLJ-1603) or pending validity of the inc' approach.

Patch: CLJ-1515-deftype3.patch
Screened by: Alex Miller

(range) and supports auto-promotion towards infinity in this patch, which seems to be implied by the doc string but was not actually implemented or tested correctly afaict.

Comment by Alex Miller [ 29/Aug/14 3:19 PM ]

1) Not sure about losing chunked seqs - that would make older usage slower, which seems undesirable.
2) RangeIterator.next() needs to throw NoSuchElementException when walking off the end
3) I think Range should implement IReduce instead of relying on support for CollReduce via Iterable.
4) Should let _hash and _hasheq auto-initialize to 0 not set to -1. As is, I think _hasheq always would be -1?
5) _hash and _hasheq should be transient.
6) count could be cached (like hash and hasheq). Not sure if it's worth doing that but seems like a win any time it's called more than once.
7) Why the change in test/clojure/test_clojure/serialization.clj ?
8) Can you squash into a single commit?

Comment by Timothy Baldridge [ 29/Aug/14 3:40 PM ]

1) I agree, adding chunked seqs to this will dramatically increase complexity, are we sure we want this?
2) exception added
3) I can add IReduce, but it'll pretty much just duplicate the code in protocols.clj. If we're sure we want that I'll add it too.
4) fixed hash init values, defaults to -1 like ASeq
5) hash fields are now transient
6) at the cost of about 4 bytes we can cache the cost of a multiplication and an addition, doesn't seem worth it?
7) the tests in serialization.clj assert that the type of the collection roundtrips. This is no longer the case for range which starts as Range and ends as a list. The change I made converts range into a list so that it properly roundtrips. My assumption is that we shouldn't rely on all implementations of ISeq to properly roundtrip through EDN.
8) squashed.

Comment by Alex Miller [ 29/Aug/14 3:49 PM ]

6) might be useful if you're walking through it with nth, which hits count everytime, but doubt that's common
7) yep, reasonable

Comment by Andy Fingerhut [ 18/Sep/14 6:52 AM ]

I have already pointed out to Edipo in personal email the guidelines on what labels to use for Clojure JIRA tickets here: http://dev.clojure.org/display/community/Creating+Tickets

Comment by Timothy Baldridge [ 19/Sep/14 10:02 AM ]

New patch with IReduce directly on Range instead of relying on iterators

Comment by Alex Miller [ 01/Oct/14 2:00 PM ]

The new patch looks good. Could you do a test to determine the perf difference from walking the old chunked seq vs the new version? If the perf diff is negligible, I think we can leave as is.

Another idea: would it make sense to have a specialized RangeLong for the (very common) case where start, end, and step could all be primitive longs? Seems like this could help noticeably.

Comment by Timothy Baldridge [ 03/Oct/14 10:00 AM ]

Looks like chunked seqs do make lazy seq code about 5x faster in these tests.

Comment by Ghadi Shayban [ 03/Oct/14 10:22 AM ]

I think penalizing existing code possibly 5x is a hard cost to stomach. Is there another approach where a protocolized range can live outside of core? CLJ-993 has a patch that makes it a reducible source in clojure.core.reducers, but it's coll-reduce not IReduce, and doesn't contain an Iterator. Otherwise we might have to take the chunked seq challenge.

Alex: Re long/float. Old reified Ranged.java in clojure.lang blindly assumes ints, it would be nice to have a long vs. float version, though I believe the contract of reduce boxes numbers. (Unboxed math can be implemented very nicely as in Prismatic's Hiphip array manipulation library, which takes the long vs float specialization to the extreme with different namespaces)

Comment by Timothy Baldridge [ 03/Oct/14 10:38 AM ]

I don't think anyone is suggesting we push unboxed math all the way down through transducers. Instead, this patch contains a lot of calls to Numbers.*, if we were to assume that the start end and step params of range are all Longs, then we could remove all of these calls and only box when returning an Object (in .first) or when calling IFn.invoke (inside .reduce)

Comment by Alex Miller [ 03/Oct/14 10:46 AM ]

I agree that 5x slowdown is too much - I don't think we can give up chunked seqs if that's the penalty.

On the long case, I was suggesting what Tim is talking about, in the case of all longs, create a Range that stores long prims and does prim math, but still return boxed objects as necessary. I think the only case worth optimizing is all longs - the permutation of other options gets out of hand quickly.

Comment by Ghadi Shayban [ 03/Oct/14 11:00 AM ]

Tim, I'm not suggesting unboxed math, but the singular fast-path of all-Longs that you and Alex describe. I mistakenly lower-cased Long/Float.

Comment by Timothy Baldridge [ 31/Oct/14 11:30 AM ]

Here's the latest work on this, a few tests fail. If someone wants to take a look at this patch feel free, otherwise I'll continue to work on it as I have time/energy.

Comment by Nicola Mometto [ 14/Nov/14 12:51 PM ]

As discussed with Tim in #clojure, the current patch should not change ArrayChunk's reduce impl, that's an error.

Comment by Alex Miller [ 09/Dec/14 2:40 AM ]

Still a work in progress...

Comment by Nicola Mometto [ 09/Dec/14 8:44 AM ]

Alex, while this is still a work in progress, I see that the change on ArrayChunk#reduce from previous WIP patches not only has not been reverted but has been extended. I don't think the current approach makes sense as ArrayChunk#reduce is not part of the IReduce/IReduceInit contract but of the IChunk contract and changing the behaviour to be IReduce-like in its handling of reduced introduces the burden of having to use preserve-reduced on the reducing function to no apparent benefit.

Given that the preserve-reduced is done on the clojure side, it seems to me like directly invoking .reduce rather than routing through internal-reduce should be broken but I haven't tested it.

Comment by Alex Miller [ 09/Dec/14 9:49 AM ]

That's the work in progress part - I haven't looked at yet. I have not extended or done any work re ArrayChunk, just carried through what was on the prior patch. I'll be working on it again tomorrow.

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

I am impressed and have learned a ton through this exercise.

quick review of clj-1515-2
1) withMeta gives the newly formed object the wrong meta.
2) LongRange/create() is the new 0-arity constructor for range, which sets the 'end' to Double/POSITIVE_INFINITY cast as a long. Current core uses Double/POSITIVE_INFINITY directly. Not sure how many programs rely upon iterating that far, or how they would break.
3) Relatedly, depending on the previous point: Because only all-long arguments receive chunking, the very common case of (range) with no args would be unchunked. Doesn't seem like too much of a stretch to add chunking to the other impl.
4) Though the commented invariants say that Range is never empty, the implementation uses a magic value of _count == 0 to mean not cached, which is surprising to me. hashcodes have the magic value of -1
5) s/instanceof Reduced/RT.isReduced
6) is the overflow behavior of "int count()" correct?

Comment by Alex Miller [ 11/Dec/14 12:06 AM ]

1) agreed!
2) Good point. I am definitely changing behavior on this (max of 9223372036854775807). I will look at whether this can be handled without affecting perf. Really, handling an infinite end point is not compatible with several things in LongRange.
3) I actually did implement chunking for the general Range and found it was slower (the original Clojure chunking is faster). LongRange is making up for that difference with improved primitive numerics.
4) Since empty is invalid, 0 and -1 are equally invalid. But I agree -1 conveys the intent better.
5) agreed
6) probably not. ties into 2/3.

Thanks for this, will address.

Comment by Alex Miller [ 11/Dec/14 12:11 AM ]

Added -4 patch that addresses 1,4,5 but not the (range) stuff.

Comment by Alex Miller [ 11/Dec/14 12:51 PM ]

Latest -7 patch addresses all feedback and perf #s updated.

Comment by Ghadi Shayban [ 22/Dec/14 10:59 PM ]

See CLJ-1572, I believe CollReduce needs to be extended directly to clojure.lang.{LongRange,Range} inside protocols.clj

Comment by Ghadi Shayban [ 29/Dec/14 12:29 PM ]

Seems like a missing benchmark is (range 0 31) without the doall. Current patch (-9) allocates the chunk buffer at range's construction time. Maybe this can be delayed?

Comment by Alex Miller [ 02/Jan/15 11:24 AM ]

Ghadi - you are right. I reworked the patch (new -10 version) so that the chunk is only created on demand. Basically, the chunking is only used when traversing via chunked seqs and in normal seq iteration or reduce, no explicit chunks are created. That improved several timings. I also added the bench for (range 0 31) which was greatly improved by lazily creating the chunk, so good point on that.

Comment by Michael Blume [ 03/Jan/15 1:38 PM ]

I'm looking at the implementation of equiv() and wondering if it's worth checking whether the other object is also a reified range and comparing the private parameters rather than iterating through the sequences.

Comment by Ghadi Shayban [ 18/Jan/15 7:26 PM ]

Attaching a simplified implementation as a deftype. I benchmarked a billion variants and dumped bytecode. I'm attaching the best performing combination, and it beats out the Java one in most cases.

Two deftypes, one specialized to primitive longs, the other generic math.
Implement: Seqable/Counted,IReduce,IReduceInit,Iterable
Also, marker interfaces Sequential and Serialized.

The implementations are very straightforward, but Seqable deserves mention.
'seq' delegates its implementation to the existing lazy-seq based range, which has been stripped of the strange corner cases and now only handles strictly ascending/descending ranges. It has been renamed range*, rather than range1 because all the arguments to range are already numbers. core references to range have been accordingly renamed.

The new range constructor is loaded after reduce & protocols load. It checks types, and delegates to either long-range or generic-range, which handle the wacky argument cases that are not strictly ascending or descending. I'm annoyed with the structural duplication of those conditionals, not sure how to solve it.

Inside the LongRange implementation of IReduce/IReduceInit, boxing is carefully controlled, and was verified through bytecode dumping.

I elaborated the benchmarks for comparison, and also included benchmarking without type specialization.

You discover strange things working on this stuff. Turns out having the comparator close over the 'end' field is beneficial: #(< % end).

Alex, I figured out why the Java versions had a nearly exactly 2x regression on (doall (range 0 31)), and I attached a change to 'dorun'. Instead of calling (seq coll) then (next coll), it effectively calls (next (seq coll)). I think the implementation assumes that calling 'seq' is evaluating the thunk inside LazySeq. This should also help other Seqable impls like Cycle/Iterate/Repeat CLJ-1603.

Results (criterium full runs):

code 1.7.0-alpha5 clj-1515-10.patch (Java) deftype specialized (attached) deftype unspecialized
(count (filter odd? (take (* 1024 1024) (range)))) 187.30 ms 194.92 ms 182.53 ms 184.13 ms
(transduce (take (* 1024 1024)) + (range)) 58.27 ms 89.37 ms 84.69 ms 84.72 ms
(count (range (* 1024 1024))) 62.34 ms 27.11 ns not run not run
(reduce + (map inc (range (* 1024 1024)))) 57.63 ms 46.14 ms 46.17 ms 52.94 ms
(reduce + (map inc (map inc (range (* 1024 1024))))) 82.83 ms 68.66 ms 64.36 ms 71.76 ms
(count (keep odd? (range (* 1024 1024)))) 76.09 ms 57.59 ms not run not run
(transduce (comp (map inc) (map inc)) + (range (* 1024 1024))) 52.17 ms 39.71 ms 28.83 ms 42.23 ms
(reduce + 0 (range (* 2048 1024))) 85.93 ms 38.03 ms 26.49 ms 42.43 ms
(doall (range 0 31)) 1.33 µs 2.89 µs 1.03 µs 1.08 µs
(doall (range 0 32)) 1.35 µs 2.97 µs 1.07 µs 1.10 µs
(doall (range 0 128)) 5.27 µs 11.93 µs 4.19 µs 4.29 µs
(doall (range 0 512)) 21.66 µs 47.33 µs 16.95 µs 17.66 µs
(doall (range 0 4096)) 171.30 µs 378.52 µs 135.45 µs 140.27 µs
(into [] (map inc (range 31))) 1.97 µs 1.57 µs 1.87 µs 1.95 µs
(into [] (map inc) (range 31)) 1.66 µs 824.67 ns 891.90 ns 1.11 µs
(into [] (range 128)) 5.11 µs 2.21 µs 2.43 µs 3.36 µs
(doall (range 1/2 1000 1/3)) 1.53 ms 1.67 ms 1.59 ms 1.52 ms
(doall (range 0.5 1000 0.33)) 164.83 µs 382.38 µs 149.38 µs 141.21 µs
(into [] (range 1/2 1000 1/3)) 1.53 ms 1.40 ms 1.43 ms 1.50 ms
(into [] (range 0.5 1000 0.33)) 157.44 µs 108.27 µs 104.18 µs 127.20 µs

Open Questions for screeners of the deftype patch:

1) What to do about the autopromotion at the Long/MAX_VALUE boundary? I've preserved the current behavior of Clojure 1.6.
2) Alex, I did not pull forward the filter chunk tweak you discovered
3) Is the structural duplication of the conditionals in generic-range long-range awful?
4) Are there any other missing interfaces? IMeta comes to mind. Not sure about IHashEq either.

Comment by Ghadi Shayban [ 18/Jan/15 7:30 PM ]

note with the deftype patch, the transduce over infinite (range) case is the only one where 'master' currently performs best. This is because the implementation was changed to (iterate inc' 0). When CLJ-1603 is applied that situation should improve better.

Comment by Ghadi Shayban [ 18/Jan/15 8:27 PM ]

fixup patch CLJ-1515-deftype

Comment by Ghadi Shayban [ 18/Jan/15 11:05 PM ]

new patch CLJ-1515-deftype-nostructural-dup.patch with the silly conditional duplication removed. Updated benchmarks including 'count' impls. These benchmarks run on a different machine with the same hardness. Blue ribbon.

code 1.7.0-alpha5 1.7.0-rangejava 1.7.0-rangespecial rangespecial / alpah5
(count (filter odd? (take (* 1024 1024) (range)))) 297.14 ms 333.93 ms 328.62 ms 1.10
(transduce (take (* 1024 1024)) + (range)) 105.55 ms 145.44 ms 164.70 ms 1.56
(count (range (* 1024 1024))) 108.92 ms 61.09 ns 26.61 ns 0
(reduce + (map inc (range (* 1024 1024)))) 97.67 ms 95.41 ms 84.62 ms 0.86
(reduce + (map inc (map inc (range (* 1024 1024))))) 140.21 ms 135.59 ms 116.38 ms 0.83
(count (keep odd? (range (* 1024 1024)))) 121.18 ms 104.63 ms 111.46 ms 0.92
(transduce (comp (map inc) (map inc)) + (range (* 1024 1024))) 100.40 ms 86.28 ms 67.17 ms 0.67
(reduce + 0 (range (* 2048 1024))) 131.77 ms 80.43 ms 63.24 ms 0.48
(doall (range 0 31)) 2.53 µs 4.36 µs 2.24 µs 0.89
(doall (range 0 32)) 2.37 µs 4.00 µs 1.99 µs 0.84
(doall (range 0 128)) 9.20 µs 14.98 µs 8.01 µs 0.87
(doall (range 0 512)) 37.28 µs 59.13 µs 35.16 µs 0.94
(doall (range 0 4096)) 331.28 µs 471.57 µs 291.76 µs 0.88
(into [] (map inc (range 31))) 2.83 µs 2.79 µs 2.67 µs 0.94
(into [] (map inc) (range 31)) 2.21 µs 1.39 µs 1.26 µs 0.57
(into [] (range 128)) 6.72 µs 3.25 µs 3.09 µs 0.46
(doall (range 1/2 1000 1/3)) 3.41 ms 4.04 ms 3.14 ms 0.92
(doall (range 0.5 1000 0.33)) 281.04 µs 530.92 µs 244.14 µs 0.87
(into [] (range 1/2 1000 1/3)) 3.32 ms 3.71 ms 2.99 ms 0.90
(into [] (range 0.5 1000 0.33)) 215.53 µs 165.93 µs 138.86 µs 0.64
Comment by Alex Miller [ 19/Jan/15 8:32 AM ]

This is looking very good and I think we should move forward with it as the preferred approach. Feel free to update the description appropriately. I'll file a separate ticket with the filter tweak. Some comments on the patch:

1) GenericRange/count - this math is broken if you start to mix infinity in there. I think just (count (seq this)) is safer.
2) GenericRange/iterable - I think if we are IReduceInit and Seqable, we can omit this. I had it in the Java one because I inherited Iterable via ASeq but that's not an issue in this impl.
3) LongRange/reduce - why Long/valueOf? Isn't start a long field?
4) LongRange/iterable - ditto #2
5) print-method - anytime I see print-method, there should probably be print-dup too. For example, this is broken: (binding [*print-dup* true] (println (range 0 10)))
6) Is serialization broken by this patch? Can you justify the test changes?

Comment by Ghadi Shayban [ 19/Jan/15 12:51 PM ]

regarding Serialization tests currently:

(defn roundtrip
  (let [rt (-> v serialize deserialize)
        rt-seq (-> v seq serialize deserialize)]        ;; this
    (and (= v rt)            ;; this fails because the test ^ calls seq first
      (= (seq v) (seq rt))   ;; this passes
      (= (seq v) rt-seq))))  ;; this passes

These new types are merely seqable, so (not= (LongRange. 0 10 1) (seq (LongRange. 0 10 1)))

Not sure how to handle this 100%. Nothing precludes the LongRange itself from roundtripping, but just that calling seq on it returns a separate object.

Comment by Alex Miller [ 23/Jan/15 9:57 AM ]

More comments...

1) Instead of extending IReduce and IReduceInit, just extend IReduce and implement both arities (IReduce extends IReduceInit).
2) I'm slightly troubled by the .invokePrim now. Did you look at a macro that does prim type-hinting maybe?
3) On the serialization thing, is the problem really with serialization or with equality? The test that's failing is (= v rt). Because these are not IPersistentCollections, pcequiv won't be used and it's just calling LongRange.equals(), which is not implemented and falls back to identity, right? Probably need to implement the hashCode and equals stuff. Might need IHashEq too.

user=> (= (range 5) (range 5))
Comment by Ghadi Shayban [ 23/Jan/15 12:13 PM ]

Took care of 1) and 3). Punting on hiding the invokePrim behind a macro. It may be shameful but it works really fast.

Another case found:
(assoc {'(0 1 2 3 4) :foo} (range 5) :bar) needs to properly overwrite keys in the map.

Cause: Might be irrelevant in the face of CLJ-1375. Util/equiv for maps doesn't use IPC/equiv unless the collection is also java.util.Collection or Map. If it is then it checks for IPersistentCollection. I added a check for IPersistentCollection first.


To get hasheq working, I added back the iterators for use by Murmur3/hashOrdered.

Comment by Ghadi Shayban [ 23/Jan/15 3:20 PM ]

Handle hash and equality not through IPersistentCollection. w/ test-cases too.

[CLJ-1642] Add mention of new :warn-on-boxed option to doc string of unchecked-math Var Created: 15/Jan/15  Updated: 15/Jan/15

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

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

Attachments: Text File clj-1642-v1.patch    
Patch: Code
Approval: Screened


A small doc string enhancement about the new compiler behavior in Clojure 1.7 when *unchecked-math* is bound to :warn-on-boxed


Patch: clj-1642-v1.patch
Screened by: Alex Miller

Comment by Andy Fingerhut [ 15/Jan/15 11:51 AM ]

Patch clj-1642-v1.patch dated Jan 15 2014 is one way to document the new :warn-on-boxed behavior.

[CLJ-1635] Make distinct/dedupe/interpose transducer tests play nicely with new reporting Created: 09/Jan/15  Updated: 16/Jan/15

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

Type: Enhancement Priority: Minor
Reporter: Michael Blume Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: test, transducers

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


Fix interaction between CLJ-1601 (which introduced new transducers) and CLJ-1621 (which improved transducer tests) to improve test reporting for these new transducer arities as well.

Note from Alex M: I goofed these while rebasing CLJ-1601 after CLJ-1621 went in.

Patch: clj-1635.patch

Screened by: Alex Miller

Comment by Alex Miller [ 09/Jan/15 2:31 PM ]

My fault in the integration process! We'll try to get it fixed in 1.7. Thanks...

Generated at Mon Jan 26 04:32:15 CST 2015 using JIRA 4.4#649-r158309.