Converted from http://www.assembla.com/spaces/clojure/tickets/21

Converted from http://www.assembla.com/spaces/clojure/tickets/47

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

aredington said: This appears to still be a problem with the generated bytecode in 1.3.0. Examining the bytecode for last, the problem has moved to invokeStatic:

<pre>
public static java.lang.Object invokeStatic(java.lang.Object) throws java.lang.Exception;
Code:
0: aload_0
1: invokestatic #50; //Method clojure/core$next.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
4: dup
5: ifnull 25
8: getstatic #56; //Field java/lang/Boolean.FALSE:Ljava/lang/Boolean;
11: if_acmpeq 26
14: aload_0
15: invokestatic #50; //Method clojure/core$next.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
18: astore_0
19: goto 0
22: goto 30
25: pop
26: aload_0
27: invokestatic #59; //Method clojure/core$first.invokeStatic:(Ljava/lang/Object;)Ljava/lang/Object;
30: areturn
</pre>

Line number 22 is an unreachable goto given the prior goto on line 19.

Converted from http://www.assembla.com/spaces/clojure/tickets/64

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

cemerick said: A patch has been submitted (via ticket #174) to add Serializable support to c.l.Keyword.

Converted from http://www.assembla.com/spaces/clojure/tickets/77

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

Converted from http://www.assembla.com/spaces/clojure/tickets/115

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

Converted from http://www.assembla.com/spaces/clojure/tickets/126

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

hiredman said: Related association with ticket #259 was added

Converted from http://www.assembla.com/spaces/clojure/tickets/127

jmcconnell said: I'd be happy to take this up with the GAE folks if it winds up looking like this is something they should probably allow or if we need any further information from them on their policies.

richhickey said: Updating tickets (#127, #128, #129, #130)

mikehinchey said: GAE made some changes a few weeks ago, maybe changed this because I'm able to load from .clj files now (not the servlet, of course, which must be gen-class).

richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)

mattrevelle said: J. McConnell, was this issue resolved by a change in GAE policy?

Converted from http://www.assembla.com/spaces/clojure/tickets/150

Converted from http://www.assembla.com/spaces/clojure/tickets/153

Converted from http://www.assembla.com/spaces/clojure/tickets/140

Converted from http://www.assembla.com/spaces/clojure/tickets/213
Attachments:
invariants.patch - https://www.assembla.com/spaces/clojure/documents/dd4kUS3MWr3QvMeJe5aVNr/download/dd4kUS3MWr3QvMeJe5aVNr

Converted from http://www.assembla.com/spaces/clojure/tickets/233

chouser@n01se.net said: Stuart, I don't see a patch attached.

Converted from http://www.assembla.com/spaces/clojure/tickets/252

Converted from http://www.assembla.com/spaces/clojure/tickets/277

bpsm said: I've attached a patch to #410, which also fixes this issue. (In fact, it turns out that it's the same patch tlj previously attached here.)

stu said: Duplicated association with ticket #410 was added

Converted from http://www.assembla.com/spaces/clojure/tickets/319
Attachments:
TransactionalHashMap.java.patch - https://www.assembla.com/spaces/clojure/documents/cuuZnsuuWr36H0eJe5dVir/download/cuuZnsuuWr36H0eJe5dVir

megabyte2021 said: [file:cuuZnsuuWr36H0eJe5dVir]: The patch file

stu said: Please add a test case.

Converted from http://www.assembla.com/spaces/clojure/tickets/434

Converted from http://www.assembla.com/spaces/clojure/tickets/459

Converted from http://www.assembla.com/spaces/clojure/tickets/148

scgilardi said: It's saying that the symbol com.howardlewisship.cascade.test-views/write already resolves to #���clojure.contrib.pprint/write, so you can't def a new write in com.howardlewisship.cascade.test-views.

What do you propose for an alternate wording of the error message here?

I think the issue is that only one side of the conflict is reported in the error, so if you get this kind of error in the middle of a large project it can be hard to figure out which namespace is conflicting. Take a toy example:

user=> (ns foo)
foo=> (def foobar 42)
foo=> (ns bar)
bar=> (def foobar 0)
bar=> (ns problem)
problem=> (refer 'foo)
problem=> (refer 'bar)
java.lang.IllegalStateException: foobar already refers to: #'foo/foobar in namespace: problem (NO_SOURCE_FILE:0)

In this case it would be best if the error said something like:

"Conflict referring to #'bar/foobar in #<Namespace problem> because foobar already refers to: #'foo/foobar."

This way the error message clearly identifies the location of the conflict, and the locations of the two conflicting vars.

Hopefully this helps clarify. I think I see where to fix it in warnOrFailOnReplace on line 88 of src/jvm/clojure/lang/Namespace.java, and this reminds me I need to send in a CA so I can pitch in next time...

It looks like the true conflict is in test-views, not in dom. A small example of the line number breakage showing the problem on master (1.3) would be very helpful.

Converted from http://www.assembla.com/spaces/clojure/tickets/272

stu said: Suggestions from Volkan Yazici:

Hi,

I saw your "load/ns/require/use overhaul" ticket[1] and would like to
ask for a few extra overhaulings. I have a project called retop, and
here is its file hiearachy:

tr/edu/bilkent/cs/retop.clj
tr/edu/bilkent/cs/retop/km.clj
tr/edu/bilkent/cs/retop/graph.clj
tr/edu/bilkent/cs/retop/main.clj
tr/edu/bilkent/cs/retop/util.clj

In retop.clj, I have below ns definition.

(ns tr.edu.bilkent.cs.retop
(:gen-class)
(:import
(com.sun.jna
Function
Pointer)
(com.sun.jna.ptr
IntByReference)
(tr.edu.bilkent.cs.patoh
HyperGraph
HyperGraphException
Parititoning
ParititoningParameters))
(:load
"retop/util"
"retop/km"
"retop/graph"
"retop/main"))

And in every .clj file in retop/ directory I have below in-ns in the
very first line.

(in-ns 'tr.edu.bilkent.cs.retop)

The problems with the ns decleration are:

1) Most of the :import's in retop.clj only belong to a single .clj file.
For instance,

(tr.edu.bilkent.cs.patoh
HyperGraph
HyperGraphException
Parititoning
ParititoningParameters)

imports are only used by graph.clj. Yep, I can add an (import ...)
line just after the (in-ns ...), but wouldn't it be better if I can
specify that in (in-ns ...) form?

2) See (:load ...) clause in (ns ...) form. There are lots of
unnecessary directory prefixes. I'd be prefer something ala Common
Lisp's defpackage:

(:load
"packages" ; packages.clj
("retop"
"util" ; retop/util.clj
"km" ; retop/km.clj
"graph" ; retop/graph.clj
("graph"
"foo" ; retop/graph/foo.clj
"bar) ; retop/graph/bar.clj
"main")) ; retop/main.clj

Also, being able to use wildcards would be awesome.

3) There are inconsistencies between macros and functions. For instance,
consider:

(ns foo.bar.baz (:use mov))
(in-ns 'foo.bar.baz)
(use 'mov)

I'd like to get rid of quotations in both cases.

I'm not sure if I'm using the right tools and doing the right approach
for such a project. But if you agree with the above overhauling
requirements, I'd like to see them appear in the same assembla ticket as
well.

stuart.sierra said: My requests:

1. If writing macros that do not evaluate their arguments, provide function versions that do evaluate their arguments.

2. Do not support prefix lists for loading Clojure namespaces. It's hard to parse with external tools.

3. Do not conflate importing Java classes with loading Clojure namespaces. They are fundamentally different operations with different semantics.

I have implemented some ideas in a macro called "need" at http://github.com/stuartsierra/need

Further requests:

Permit tools to read the "ns" declaration and statically determine the dependencies of a namespace, without evaluating any code.

Permit tools to read the "ns" declaration and statically determine the dependencies of a namespace, without evaluating any code.

This would be great for building OSGi bundles where Bnd is currently not much help.

Converted from http://www.assembla.com/spaces/clojure/tickets/400
Attachments:
flatten-enhancement.diff - https://www.assembla.com/spaces/clojure/documents/c5chtAJQir353NeJe5cbLr/download/c5chtAJQir353NeJe5cbLr

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

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

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

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

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

Converted from http://www.assembla.com/spaces/clojure/tickets/450

I independently wanted this. Here's a patch for: some, not-any?, every?, not-every?. If this is roughly what's wanted I'll be happy to add filter, remove, take-while, drop-while.

Note that there are a few cases of (every? identity ...) and (some identity ...) in core.clj itself; the patch removes "identity" from those.

clj-450-add-default-pred-arg-to-core-fns-patch.txt dated Apr 26 2012 is identical to Jason Orendorff's, except it is in git format. Jason is not on the list of Clojure contributors as of today. I have sent him an email asking if he has done so, or is planning to.

Of course I'd be happy to send in a contributor agreement. ...Is there actually any interest in taking this patch or something like it?

I don't know if there is any interest in taking this patch. Perhaps a Clojure screener will take a look at it and comment, but I am not a screener and can't promise anything.

Converted from http://www.assembla.com/spaces/clojure/tickets/451

Converted from http://www.assembla.com/spaces/clojure/tickets/396

Converted from http://www.assembla.com/spaces/clojure/tickets/405

stu said: This could be fixed with an assert-valid-defrecord call in core_deftype, similar to assert-valid-fdecl in core.clj. Such a function would also be a place to hang other defrecord error messages.

Converted from http://www.assembla.com/spaces/clojure/tickets/379
Attachments:
ticket-379-fix.diff - https://www.assembla.com/spaces/clojure/documents/c5XWHcD4yr34HveJe5ccaP/download/c5XWHcD4yr34HveJe5ccaP

alexott said: possible fix is attached

alexott said: [file:c5XWHcD4yr34HveJe5ccaP]

Converted from http://www.assembla.com/spaces/clojure/tickets/130

richhickey said: Updating tickets (#127, #128, #129, #130)

juergenhoetzel said: This is still a issue on

Clojure 1.2.0-master-SNAPSHOT

Any progress, hints? I prefer interactive documentiation via slime/repl

Converted from http://www.assembla.com/spaces/clojure/tickets/401

Converted from http://www.assembla.com/spaces/clojure/tickets/112

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

Converted from http://www.assembla.com/spaces/clojure/tickets/440

Patch adds support for varargs. Builds on top of patch in CLJ-445.

Patch updated to current CLJ-445 patch.

Is this ticket on hold? I find myself typing (.someCall arg1 arg2 (into-array SomeType nil)) alot just to get the right method to be called. This ticket sounds like it would address that extraneous into-array arg that I use alot.

fixbug445.diff uploaded on Oct 29 2012 was written Oct 23 2010 by Alexander Taggart. I am simply copying it from the old Assembla ticket tracking system to here to make it more easily accessible. Not surprisingy, it doesn't apply cleanly to latest master. I don't know how much effort it would be to update it, but only a few hunks do not apply cleanly according to 'patch'. See the "Updating stale patches" section on the JIRA workflow page here: http://dev.clojure.org/display/design/JIRA+workflow

Ugh. Deleted the attachment because it was for CLJ-445, or at least it was named that way. CLJ-445 definitely has a long comment history, so if one or more of its patches address this issue, then you can read the discussion there to see the history.

I don't know of any "on hold" status for tickets, except for one or two where Rich Hickey has explicitly said in a comment that he wants to wait a while before making the change. There are just tickets that contributors choose to work on and ones that screeners choose to screen.

Converted from http://www.assembla.com/spaces/clojure/tickets/19

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

this seems like a great task for someone just starting out contributing to clojure.

Converted from http://www.assembla.com/spaces/clojure/tickets/274

donmullen said: Updated each run to [_] for new syntax.

Now gives exception listed.

richhickey said: We're not going to allow closing over mutable fields. Instead we'll have to generate something other than fn for loops et al used as expressions. Not going to come before cinc

Converted from http://www.assembla.com/spaces/clojure/tickets/15

Wouldn't the naive approach incur realizing lazy sequences when adding them to a list or a vector or as values in a map?

Converted from http://www.assembla.com/spaces/clojure/tickets/259

richhickey said: How about some sample case that demonstrates the problem?

hiredman said: Related association with ticket #126 was added

Linux herberteuler 3.2.0-2-amd64 #1 SMP Sat May 12 23:08:28 UTC 2012 x86_64 GNU/Linux

The second behavior should be in Clojure 1.3:

$ clojure1.3 ~/tmp/proxy-bug.clj
Exception in thread "main" clojure.lang.ArityException: Wrong number of args (4) passed to: user$eval1$fn
at clojure.lang.AFn.throwArity(AFn.java:437)
at clojure.lang.AFn.invoke(AFn.java:51)
at user.proxy$java.io.InputStream$0.read(Unknown Source)
at user$eval1.invoke(proxy-bug.clj:9)
at clojure.lang.Compiler.eval(Compiler.java:6468)
at clojure.lang.Compiler.load(Compiler.java:6905)
at clojure.lang.Compiler.loadFile(Compiler.java:6866)
at clojure.main$load_script.invoke(main.clj:282)
at clojure.main$script_opt.invoke(main.clj:342)
at clojure.main$main.doInvoke(main.clj:426)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.lang.Var.invoke(Var.java:401)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.Var.applyTo(Var.java:518)
at clojure.main.main(main.java:37)

Sorry for the inconvenience.

The problem occurs in 1.2 but is fixed on master. Leaving in backlog in case we ever cut another 1.2 release--if not, then mark as fixed.

Improved via patch on CLJ-426.

(defn test-case ^double [^long i ^double d1 ^double d2]
  (case (int i)
    0 d1
    d2))

now emits as

 0  lload_1 [i]
 1  invokestatic clojure.lang.RT.intCast(long) : int [67]
 4  istore 7 [G__7903] // let-bound expression
 6  iload 7 [G__7903]
 8  tableswitch default: 32
      case 0: 28
28  dload_3 [d2]
29  goto 34
32  dload 5 [arg2]
34  dreturn

or if the int cast of the expression is omitted:

 0  lload_1 [i]
 1  lstore 7 [G__7903] // let-bound expression
 3  lload 7 [G__7903]
 5  l2i
 6  tableswitch default: 35
      case 0: 24
24  lconst_0           // match, verify long expr wasn't truncated
25  lload 7 [G__7903]
27  lcmp
28  ifne 35
31  dload_3 [d2]
32  goto 37
35  dload 5 [arg2]
37  dreturn

Clojure 1.2.0, 1.2.1, 1.3.0-alpha6, JDK 1.6.0_24, Ubuntu 10.10

The first case where we saw this was actually in having a deftype implement a Java interface (not a protocol) and in that case you cannot extend the interface outside the deftype (although comparing based on class name of course works).

All

diff.patch of Nov 27, 2011 does not apply cleanly to latest master version of Clojure code (using "patch -p1 < diff.patch", at least). It is preferred by Clojure team that patches are in git format-patch format. Instructions for producing such a patch are given at http://clojure.org/patches

Rahul, are you planning to sign a Clojure Contributor Agreement? Without that, this code cannot be included in Clojure, unless a contributor reimplements it on their own.

In private communication with the patch author today, he expressed an interest in submitting a signed CA so this patch can be considered for inclusion in Clojure.

Reproduced on Ubuntu using Sun Java 1.6, OpenJDK 1.6, and Sun Java 1.7

Jeff, this occurs in any Clojure program where certain threading mechanisms are invoked. In your case, clojure.java.shell/sh uses future, which causes threads to be created that then sit around for about 60 seconds after everything else quits, and the main Java process does not exit until that happens. Another way to get the program to exit quickly is to call (shutdown-agents), but that isn't any more convenient than (System/exit 0), nor is there any obvious way you can tell as a newcomer that you should be doing this.

The ticket CLJ-124 is marked with status "Approved" at this time, which leads me to believe that perhaps soon there will be a change made to Clojure such that this situation will change.

This behavior is now documented on clojuredocs.org for future, and both pmap and clojure.java.shell/sh refer to future for details.

Maven using the one-jar plugin

OS X, Java 1.6.0?

Mac OS X 10.5, lein 1.7 and lein 2.0

I can't reproduce this under Clojure 1.3 or 1.4, and Leiningen 1.7.1 on either Java 1.7.0-jdk7u4-b21 OpenJDK 64-Bit or Java 1.6.0_31 Java HotSpot 64-Bit. OS is Mac OS X 10.7.

Edmund, how are you running this AOT code? I wrapped your code in a main function and built an uberjar from it.

Hi Scott,

Interesting.

I have two use cases
1. AOT compile and call from repl.
My steps: git clone, lein compile, lein repl, (use 'aots.death), (in-ns 'aots.death), (= (class (Dontwork. nil)) (class (map->Dontwork {:a 1}))) => false

2. My original use case, which I've minimised here, is an AOT ns, producing a genclass that is called instantiated from other Java (no main). This produces the same error. I will produce an example of this and post it too.

Hi Scott,

Here is an example of it failing in the interop case: https://github.com/ejackson/aotquestion2
The steps I'm following to compile this all up are

git clone git@github.com:ejackson/aotquestion2.git
cd aotquestion2/cljside/
lein uberjar
lein install
cd ../javaside/
mvn package
java -jar ./target/aotquestion-1.0-SNAPSHOT.jar

and it dies with this:

Exception in thread "main" java.lang.ClassCastException: cljside.core.Dontwork cannot be cast to cljside.core.Dontwork
at cljside.MyClass.makeDontwork(Unknown Source)
at aotquestion.App.main(App.java:8)

The error message is really confusing (to me, anyway), but I think its the same root problem as for the REPL case.

What do you see when you run the above ?

Ah, yes, looks like my initial attempt to reproduce was too simplistic. I used your second git repo, and can now confirm that it's failing for me with the same error.

I looked into this a little further and the AOT generated code looks correct, in the sense that both code paths appear to be returning the same type.

However, I wonder if this is really a ClassLoader issue, whereby two definitions of the same class are being loaded at different times, because that would cause the x.y.Class cannot be cast to x.y.Class exception that we're seeing here.

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

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

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

This issue is isolated to mvn test afaik.

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

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

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

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

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

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

Note that this would require PersistentTreeMap to implement IEditableCollection as well.

Found a work-around:

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

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

I have attached a patch that changes clojure.lang.Var and clojure.core/thread-bound? to only return true if a Var is set!-able.

Should I have made this implement Seqable as well? It wasn't clear to me, because as far as I could see this was the only function in clojure.core.reducers that's generating a brand-new sequence rather than transforming an existing one.

Previous version neglected to include the seed value of the iteration in the reduce.

Currying iterate seems useless, albeit not harmful.

While implementing repeat, I couldn't use currying. Because 1-arity is already reserved for infinite repeat ([n x] and [x], not [n x] and [n] if currying)

How about we just support currying for functions where last param is reducible?

This new patch replaces the previous patch. As requested, I am splitting up the large issue CLJ-993 into smaller tickets.

Does not depend on any of my other reducer patches, but there will probably be some minor merge conflicts unless it is merged after CLJ-1045 and CLJ-1046, and before CLJ-993.

Questions posed on the clojure-dev list around how this impacts bit-shift-left: http://groups.google.com/group/clojure-dev/browse_thread/thread/2191cbf0048d8ca6

Patch on CLJ-445 fixes this as well.

This patch fails a test around bit-shifting a BigInt: `(bit-shift-left 1N 10000)`. The reason is that the patch changes the dispatch of (BigInt, Long) from (Object, Object) to (long, int).

Clearly this can't be applied (unless another change makes it possible), but I'm putting it up as a start of the conversation.

My comment from the mailing list:

If the test breaks it likely means Numbers.shiftLeft(long,int) was
selected over Numbers.shiftLeft(Object,Object). Given that 1N is an
Object (one that can exceed the size of a long), the method selection
is incorrect, thus the patch is broken.

The suggestion of "simply" modifying paramArgTypeMatch is not sufficient since the mechanism for preferring one method over another lives in Compiler, and isn't smart enough to make these sorts of decisions.

Considering moving this out of Release.next - soliciting comments from Chas.

I'm afraid I don't have any particular insight into the issues involved at this point. I ran into the problem originally noted a while back, and opened the ticket at Rich's suggestion. I'm sorry if the text of the ticket led anyone down unfruitful paths…

The issues relating to bitshift are moot since the decision was made that bit-shifts are only for 32/64 bit values.

Still a valid issue, but de-prioritized as per Rich.

Modified version of original patch

Alex, would you mind attaching it with a unique file name? I know that JIRA lets us create multiple attachments with the same file name, and I know we can tell them apart by date and the account of the person who uploaded the attachment, but giving them the same name only seems to invite confusion.

Renamed updated patch to unique name

Converted from http://www.assembla.com/spaces/clojure/tickets/415

alexdmiller said: A simple example I tried for illustration:

user=> (let [a 1 b 2] (assert (= a b)))
#<CompilerException java.lang.AssertionError: Assert failed: (= a b)
 a : 1
 b : 2

fogus said: Of course it's weird if you do something like:

(let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= x y)))
java.lang.AssertionError: Assert failed: (= x y)
 x : 1
 y : 2
 z : 3
 a : 1
 b : 2
 c : 3
 (NO_SOURCE_FILE:0)
</code></pre>

So maybe it could be slightly changed to:
<pre><code>(defmacro assert
  "Evaluates expr and throws an exception if it does not evaluate to logical true."
  {:added "1.0"}
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                          (map (fn [[k# v#]] 
                                                 (when (some #{k#} form#) 
                                                   (str "\t" k# " : " v# sep#))) 
                                               ~bindings)))))))))
</code></pre>

So that. now it's just:
<pre><code>(let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= x y)))
java.lang.AssertionError: Assert failed: (= x y)
 x : 1
 y : 2
 (NO_SOURCE_FILE:0)

:f

fogus said: Hmmm, but that fails entirely for: (let [x 1 y 2 z 3 a 1 b 2 c 3] (assert (= [x y] [a c]))). So maybe it's better just to print all of the locals unless you really want to get complicated.
:f

jawolfe said: See also some comments in:

http://groups.google.com/group/clojure-dev/browse_frm/thread/68d49cd7eb4a4899/9afc6be4d3f8ae27?lnk=gst&q=assert#9afc6be4d3f8ae27

Plus one more suggestion to add to the mix: in addition to / instead of printing the locals, how about saving them somewhere. For example, the var assert-bindings could be bound to the map of locals. This way you don't run afoul of infinite/very large sequences, and allow the user to do more detailed interrogation of the bad values (especially useful when some of the locals print opaquely).

stuart.sierra said: Another approach, which I wil willingly donate:
http://github.com/stuartsierra/lazytest/blob/master/src/main/clojure/lazytest/expect.clj

There's one more tweak to fogus's last comment, which I'm actually using. You need to flatten the quoted form before you can use 'some' to check whether the local was used in the form:

(defmacro assert
  "Evaluates expr and throws an exception if it does not evaluate to logical true."
  {:added "1.0"}
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                          (map (fn [[k# v#]] 
                                                 (when (some #{k#} (flatten form#)) 
                                                   (str "\t" k# " : " v# sep#))) 
                                               ~bindings)))))))))

I am holding off on this until we have more solidity around http://dev.clojure.org/display/design/Error+Handling. (Considering, for instance, having all exceptions thrown from Clojure provide access to locals.)

When my pipe dream fades I will come back and screen this before the next release.

Why try to guess what someone wants to do with the locals (or any other context, for that matter) when you can specify a callback (see below). This would have been useful last week when I had an assertion that failed only on the CI box, where no debugger is available.

Rich, at the risk of beating a dead horse, I still think this is a good idea. Debuggers are not always available, and this is an example of where a Lisp is intrinsically capable of providing better information than can be had in other environments. If you want a patch for the code below please mark waiting on me, otherwise please decline this ticket so I stop looking at it.

(def ^:dynamic *assert-handler* nil)

(defn ^{:private true} local-bindings
  "Produces a map of the names of local bindings to their values."
  [env]
  (let [symbols (map key env)]
    (zipmap (map (fn [sym] `(quote ~sym)) symbols) symbols)))

(defmacro assert
  [x]
  (when *assert*
    (let [bindings (local-bindings &env)]
      `(when-not ~x
         (let [sep#  (System/getProperty "line.separator")
               form# '~x]
           (if *assert-handler*
             (*assert-handler* form# ~bindings)
             (throw (AssertionError. (apply str "Assert failed: " (pr-str form#) sep#
                                            (map (fn [[k# v#]] 
                                                   (when (some #{k#} (flatten form#)) 
                                                     (str "\t" k# " : " v# sep#))) 
                                                 ~bindings))))))))))

A slight improvement I made in my own version of this code: flatten does not affect set literals. So if you do (assert (some #{x} [a b c d])) the value of x will not be printed. Here's a modified flatten that does the job:

(defn symbols [sexp]
  "Returns just the symbols from the expression, including those
   inside literals (sets, maps, lists, vectors)."
  (distinct (filter symbol? (tree-seq coll? seq sexp))))

Attaching git format patch clj-415-assert-prints-locals-v1.txt of Stuart Halloway's version of this idea. I'm not advocating it over the other variations, just getting a file attached to the JIRA ticket.

Implemented range. A separate commit is attached, making iterate and range also Seqable, since I'm not sure if that's desired. Apply it or not, as you prefer.

Range should be foldable

Yep, so it should. Time for me to dig into the folding implementations!

Should I fold (har har) all of these commits into one? I don't know what is preferred on JIRA, and I also don't know whether range/iterate should be seqable or if I should just drop the second commit.

Yes, please merge these together, it's hard to see otherwise (I can barely read diffs as is . range and iterate shouldn't be novel in reducers, but just enhanced return values of core fns. The enhancement (e.g. protocol extensions) can come by requiring reducers since it can't be leveraged without it. Also, I'm not sure how I feel about an allocating protocol for 'splittable' - I've avoided it thus far.

So you want clojure.core/range to return some object (a Range), which implements Counted and Seqable (but isn't just a lazy-seq), and then inside of clojure.core.reducers I extend CollReduce and CollFold to that type? Okay, I can do that.

I don't quite follow what you mean by an allocating protocol. I see your point that my fold-by-halves which takes a function in is analogous to a protocol with a single function, but it doesn't allocate anything more than foldvec already does - I just pulled that logic out so that the fork/join fiddly work doesn't need to be repeated in everything foldable. Do you have an alternative recommendation, or is it just something that makes you uneasy and you're still thinking about?

While vector-fold allocs subvecs, the halving-fn must return a new vector, for all implementations. It's ok, I don't think it's likely to dominate (since fj needs new closures anyway). Please proceed, but keep range and iterate in core. They are sources, not transformers, and only transformers (which must be different from their seq-based counterparts) must reside in reducers. Thanks!

One big patch file is preferred, although that file may contain multiple commits if that makes the intent clearer.

When adding a patch, update the description of the ticket to indicate which file is the most recent. Leave old patch files around for historical reference.

It's looking harder than I expected to move iterate and range into core.clj. My plan was to just have them implement Seqable, which is easy enough, but currently they are actually instances of ISeq, because they inherit from LazySeq. A bunch of code all over the place (eg, to print them in the repl) depends on them being ISeq, so I can't just ignore it. To implement all of these methods (around thirty) would take a large amount of code, which can't easily be shared between Iteration, Range, and any future reducible sources that are added to core.clj.

I could write a macro like (defseq Range [start end step] Counted (count [this] ...) ...) which takes normal deftype args and also adds in implementations for ISeq, Collection, and so forth in terms of (.seq this), which will be a LazySeq. However, this seems like a somewhat awkward approach that I would be a little embarrassed to clutter up core.clj with. If anyone has a better alternative I will be pleased to hear it. In the mean time, I will go ahead with this macro implementation, in case it turns out to be the best choice.

– This patch subsumes all previous patches to this issue and to CLJ-992 –

In order to create an object which is both a lazy sequence and a
reducible source, I needed to add a macro named defseq to core_deftype.
It is basically a reimplementation of clojure.lang.LazySeq as a clojure
macro, so that I can "mix in" lazy-sequence functions into a new class
with whatever methods are needed for reducing and folding.

If we wanted, we could use this macro to implement lazy-seq in clojure instead of in java, but that's unrelated so I didn't do that in this patch.

As noted in a previous comment, defseq may not be the right approach, but this works until something better is suggested.

I accidentally included an implementation of drop-while in this patch, which I was playing around with to make sure I understood how this all works. I guess I'll leave it in for the moment, since it works and is useful, but I can remove it, or move it to a new JIRA ticket, if it's not wanted at this time.

Ok, I think this patch is officially off the rails. There must be a better way. Let's start with: touching core/deftype and reimplementing lazy-seq as a macro are off the table. The return value of range doesn't have to be a LazySeq, it has to be a lazy seq, .e.g. implement ISeq (7 methods, not 30) which it can do by farming out to its existing impl. It can also implement some new interface for use by the reducer logic. There is also still clojure.lang.Range still there, which is another approach. Please take an extremely conservative approach in these things.

Okay, thanks for the feedback - I'm glad I went into that last patch knowing it was probably wrong . I thought I would need to implement the java collection interfaces that LazySeq does, eg java.util.List, in order to avoid breaking interop functions like (defn range-list [n] (ArrayList. (range n))). If it's sufficient to implement ISeq (and thus IPersistentCollection), then that's pretty manageable.

It's still an unpleasant chunk of boilerplate for each new source, though; would you welcome a macro like defseq if I didn't put it in core_deftype? If so, it seems like it might as well implement the interop interfaces; if not, I can skip them and implement the 7 (isn't it more like 9?) methods in ISeq, IPersistentCollection, and Seqable for each new source type.

Thanks for pointing out clojure.lang.Range to me - I didn't realize we had it there. Of course with implementation inheritance it would be easy to make Range, Iteration, etc inherit from LazySeq and just extend protocols from them. But that means moving functionality out of clojure and into java, which I didn't think we'd want to do.

I'll put together a patch that just implements ISeq by hand for both of these new types, and attach it probably later today.

So I've written a patch that implements ISeq, but not the java Collections interfaces, and it mostly works but there are definitely assumptions in some parts of clojure.core and clojure.lang that assume seqs are Collections. The most obvious to me (ie, it shows up when running mvn test) is RT/toArray - it tests for Collection, but never for ISeq, implying that it's not willing to handle an ISeq that is not also a collection. Functions which rely on toArray (eg to-array and vec) now fail.

This patch subsumes all previous patches on this issue, but is not suitable for application because it leaves some failing tests behind - it is intended only for intermediate feedback.

It would be a great help if, time permitting, you could please write up the issues, challenges and options you've discovered somewhere on the dev wiki (even a simple table would be fantastic). I realize this has been a challenging task, and at this point perhaps we should opt for the more modest reducers/range and reducers/iterate and leave the two worlds separate. I'd like at some point to unify range, as there are many extant ranges it would be nice to be able to fold, as we can extant vectors.

Should r/range return something Seqable and Counted?

If so, I'll do the same for r/repeat.

I've sketched out a description of the issues and options. I'm not very familiar with the dev wiki and couldn't figure out where was the right place to put this. "release.next" seems to still be about 1.4 issues, and I don't know if it's "appropriate" to create a whole new category for this. It's available as a gist until a better home can be found for it.

Here's a single patch summing up the state Rich suggested "rolling back" to: separate r/range and r/iterate functions. I haven't heard any feedback since doing the writeup Rich asked for, so am not making any further progress at the moment; if something other than this patch is desired just let me know.

I prefer not to see the use of extend like this for new types. Perhaps this code is too DRY? Also, it does a lot in one patch which makes it hard to parse and accept. This adds Range, switches impl of vector folds etc. Can it be broken up into separate tickets that do each step that builds on the previous, e.g. one ticket could be: capture vector fold impl for reuse by similar things.

Okay, I should be able to split it up over the weekend. I'll also see about converting fold-by-halves into a function that is used by Range/Vector, rather than a function that gets extended onto them.

As requested, I have split up the large patch on this issue into four smaller tickets. The other three are: CLJ-1045, CLJ-1046, and CLJ-992.

CLJ-1045 contains the implementation of fold-by-halves, and as such this patch cannot be applied until CLJ-1045 is accepted. This ticket does not depend on the other two, but there will be minor merge conflicts if this is merged before them.

Corrected patch

Patch 0001-report-load-exception-with-file-and-line.diff fails build. Patch 0002-report-load-exception-with-file-and-line.diff applies, builds, and tests cleanly as of March 9, 2012. Hugo has signed a CA.

clj-939-report-load-exceptions-with-file-and-line-patch-v2.txt dated Oct 5 2012 is intended to be an update to Hugo Duncan's patch 0002-report-load-exceptions-with-file-and-line.diff dated Feb 25 2012. Because of Brandon Bloom's recently commited patch adding column numbers in addition to line numbers, this is not simply updating some lines of context, but I think it is correct. It would be good if Hugo could take a look at it and confirm.

Screened.

The error messages are better than what we had before. The line/column numbers are not particularly informative, probably because ns is a macro.

This patch doesn't change the reporting on any other (e.g. nested) exceptions? It looks like it might.

replicated on OpenJDK 7u9 on Ubuntu 12.04, and Hotspot 1.6.0_37 on OSX Lion

It's a little more insidious than type hinting: the compiler doesn't evaluate metadata in the binding vec.

This doesn't throw the necessary exception...

(let [^{:foo (Class/forName "not real")} bar 42]
bar)

neither this...

(let [^{gyorgy ligeti} a 42]
a)

Gyorgy Ligeti never resolves.

These two equivalent examples don't reflect:
(let [^objects a (make-array Object 2)]
(aget a 0))

(let [a ^objects (make-array Object 2)]
(aget a 0))

On only the left-hand side of a local binding, metadata on a symbol is not analyzed or evaluated.