<< Back to previous view

[CLJ-1420] ThreadLocalRandom instead of Math/random Created: 11/May/14  Updated: 29/Aug/14

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

Type: Enhancement Priority: Minor
Reporter: Linus Ericsson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math, performance
Environment:

Requires Java >=1.7!


Attachments: Text File 0001-rand-using-ThreadLocalRandom-and-tests-for-random.patch    
Patch: Code
Approval: Vetted

 Description   

The standard Math.random() is thread-safe through being declared as a synchronized static method.

The patch uses java.util.concurrent.ThreadLocalRandom which actually seems to be two times faster than the ordinary Math.random() in a simple single threaded criterium.core/bench:

The reason I investigated the function at all was to be sure random-number generation was not a bottleneck when performance testing multithreaded load generation.

If necessary, one could of course make a conditional declaration (like in fj-reducers) based on the existence of the class java.util.concurrent.ThreadLocalRandom, if Clojure 1.7 is to be compatible with Java versions < 1.7



 Comments   
Comment by Linus Ericsson [ 11/May/14 11:05 AM ]

Benchmark on current rand (clojure 1.6.0), $ java -version
java version "1.7.0_51"
OpenJDK Runtime Environment (IcedTea 2.4.4) (7u51-2.4.4-0ubuntu0.13.10.1)
OpenJDK 64-Bit Server VM (build 24.45-b08, mixed mode)

:jvm-opts ^:replace [] (ie no arguments to the JVM)

(bench (rand 10))
Evaluation count : 1281673680 in 60 samples of 21361228 calls.
Execution time mean : 43.630075 ns
Execution time std-deviation : 0.420801 ns
Execution time lower quantile : 42.823363 ns ( 2.5%)
Execution time upper quantile : 44.456267 ns (97.5%)
Overhead used : 3.194591 ns

Found 1 outliers in 60 samples (1.6667 %)
low-severe 1 (1.6667 %)
Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

Clojure 1.7.0-master-SNAPSHOT.

(bench (rand 10))
Evaluation count : 2622694860 in 60 samples of 43711581 calls.
Execution time mean : 20.474605 ns
Execution time std-deviation : 0.248034 ns
Execution time lower quantile : 20.129894 ns ( 2.5%)
Execution time upper quantile : 21.009303 ns (97.5%)
Overhead used : 2.827337 ns

Found 2 outliers in 60 samples (3.3333 %)
low-severe 2 (3.3333 %)
Variance from outliers : 1.6389 % Variance is slightly inflated by outliers

I had similar results on Clojure 1.6.0, and ran several different tests with similar results. java.util.Random.nextInt is suprisingly bad. The ThreadLocalRandom version of .nextInt is better, but rand-int can take negative integers, which would lead to some argument conversion for (.nextInt (ThreadLocalRandom/current) n) since it need upper and lower bounds instead of a simple multiplication of a random number [0,1).

CHANGE:

The (.nextDouble (ThreadLocalRandom/current) argument) is very quick, but cannot handle negative arguments. The speed given a plain multiplication is about 30 ns.

Comment by Linus Ericsson [ 11/May/14 12:44 PM ]

Added some simplistic tests to be sure that rand and rand-int accepts ratios, doubles and negative numbers of various kinds. A real test would likely include repeated generative testing, these tests are mostly for knowing that various arguments works etc.

Comment by Linus Ericsson [ 11/May/14 1:38 PM ]

0001-rand-using-ThreadLocalRandom-and-tests-for-random.patch contains the changed (rand) AND the test cases.

Comment by Alex Miller [ 11/May/14 5:45 PM ]

Clojure requires Java 1.6.0 so this will need to be reconsidered at a later date. We do not currently have any plans to bump the minimum required JDK in Clojure 1.7 although that could change of course.

Comment by Gary Fredericks [ 11/May/14 5:54 PM ]

I've always thought that the randomness features in general are of limited utility due to the inability to seed the PRNG, and that a clojure.core/rand dynamic var would be a reasonable way to do that.

Maybe both of these problems could be partially solved with a standard library? I started one at https://github.com/fredericksgary/four, but presumably a contrib library would be easier for everybody to standardize on.

Comment by Linus Ericsson [ 12/May/14 2:17 AM ]

Gary, I'm all for creating some well-thought out random-library, which could be a candidate for some library clojure.core.random if that would be useful.

Please have a look at http://code.google.com/p/javarng/ since that seems to do what you library four does (and more). Probably we could salvage either APIs, algorithms or both from this library.

I'll contact you via mail!

Comment by Gary Fredericks [ 20/Jun/14 10:21 AM ]

Come to think of it, a rand var in clojure.core shouldn't be a breaking change, so I'll just make a ticket for that to see how it goes. That should at the very least allow solving the concurrency issue with binding. The only objection I can think of is perf issues with dynamic vars?

Comment by Gary Fredericks [ 20/Jun/14 10:42 AM ]

New issue is at CLJ-1452.

Comment by Andy Fingerhut [ 29/Aug/14 4:50 PM ]

Patch 0001-rand-using-ThreadLocalRandom-and-tests-for-random.patch dated May 11 2014 no longer applied cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. It did apply cleanly before that day.

I have not checked how easy or difficult it might be to update this patch. See section "Updating Stale Patches" on this wiki page for some tips on updating patches: http://dev.clojure.org/display/community/Developing+Patches





[CLJ-5] Unintuitive error response in clojure 1.0 Created: 17/Jun/09  Updated: 18/Apr/14

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

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs

Attachments: File clj-5-destructure-error.diff     Text File CLJ-5.patch    
Patch: Code and Test
Approval: Vetted

 Description   

The following broken code:

(let [[x y] {}] x)

provides the following stack trace:

Exception in thread "main" java.lang.UnsupportedOperationException: nth not supported on this type: PersistentArrayMap (test.clj:0)
at clojure.lang.Compiler.eval(Compiler.java:4543)
at clojure.lang.Compiler.load(Compiler.java:4857)
at clojure.lang.Compiler.loadFile(Compiler.java:4824)
at clojure.main$load_script__5833.invoke(main.clj:206)
at clojure.main$script_opt__5864.invoke(main.clj:258)
at clojure.main$main__5888.doInvoke(main.clj:333)
at clojure.lang.RestFn.invoke(RestFn.java:413)
at clojure.lang.Var.invoke(Var.java:346)
at clojure.lang.AFn.applyToHelper(AFn.java:173)
at clojure.lang.Var.applyTo(Var.java:463)
at clojure.main.main(main.java:39)
Caused by: java.lang.UnsupportedOperationException: nth not supported on this type: PersistentArrayMap
at clojure.lang.RT.nth(RT.java:800)
at clojure.core$nth__3578.invoke(core.clj:873)
at user$eval__1.invoke(test.clj:1)
at clojure.lang.Compiler.eval(Compiler.java:4532)
... 10 more

The message "nth not supported on this type" while correct doesn't make the cause of the error very clear. Better error messages when destructuring would be very helpful.



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

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

Comment by Eugene Koontz [ 11/Nov/11 7:36 PM ]

Please see the attached patch which produces a (hopefully more clear) error message as shown below (given the broken code shown in the original bug report):

Clojure 1.4.0-master-SNAPSHOT
user=> (let [x 42 y 43] (+ x y))
85
user=> (let [[x y] {}] x)
UnsupportedOperationException left side of binding must be a symbol (found a PersistentVector instead).  clojure.lang.Compiler.checkLet (Compiler.java:6545)
user=>

In addition, this patch checks the argument of (let) as shown below:

user=> (let 42)
UnsupportedOperationException argument to (let)  must be a vector (found a Long instead).  clojure.lang.Compiler.checkLet (Compiler.java:6553)
Comment by Eugene Koontz [ 11/Nov/11 7:38 PM ]

Patch produced by doing git diff against commit ba930d95fc (master branch).

Comment by Eugene Koontz [ 13/Nov/11 11:24 PM ]

Sorry, this patch is wrong: it assumes that the left side of the binding is wrong - the [x y] in :

(let [[x y] {}] x)

because [x y] is a vector, when in fact, the left side is fine (per http://clojure.org/special_forms#let : "Clojure supports abstract structural binding, often called destructuring, in let binding lists".)

So it's the right side (the {}) that needs to be checked and flagged as erroneous, not the [x y].

Comment by Carin Meier [ 30/Nov/11 12:15 PM ]

Add patch better-error-for-let-vector-map-binding

This produces the following:

(let [[x y] {}] x)
Exception map binding to vector is not supported

There are other cases that are not handled by this though — like binding vector to a set

user=> (let [[x y] #{}] x)
UnsupportedOperationException nth not supported on this type: PersistentHashSet

Wondering if it might be better to try convert the map to a seq to support? Although this might be another issue.

Thoughts?

Comment by Aaron Bedra [ 30/Nov/11 7:12 PM ]

This seems too specific. Is this issue indicative of a larger problem that should be addressed? Even if this is the only case where bindings produce poor error messages, all the cases described above should be addressed in the patch.

Comment by Carin Meier [ 16/Dec/11 7:47 AM ]

Unfortunately, realized that this still does not cover the nested destructuring cases. Coming to the conclusion, that my approach above is not going to work for this.

Comment by Carin Meier [ 28/Apr/12 10:46 PM ]

File: clj-5-destructure-error.diff

Added support for nested destructuring errors

let [[[x1 y1][x2 y2]] [[1 2] {}]]
;=> UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap.
Comment by Kevin Downey [ 18/Apr/14 1:45 AM ]

I am not wild about that error message, let can destructure a map fine.

If there error message were to change, I would prefer to get something like "sequential destructing not supported on maps".

I actually like the "nth not supported" error message, because it is exactly the problem, nth, used by sequential destructuring, doesn't work on maps.

it conveys exactly what the problem is if you know how destructing works and what nth means, where as "UnsupportedOperationException let cannot destructure class clojure.lang.PersistentArrayMap" seems misleading when you are in the know





[CLJ-250] debug builds Created: 27/Jan/10  Updated: 23/Oct/13

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

Type: Enhancement Priority: Minor
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: build

Approval: Vetted
Waiting On: Rich Hickey

 Description   

This ticket includes two patches:

  1. a patch to set assert when clojure.lang.RT loads, based on the presence of system property clojure.debug
  2. expand error messages in assert to include local-bindings</code> (a new macro which wraps the implicit <code>&env)

Things to consider before approving these patches:

  1. should there be an easy Clojure-level way to query if debug is enabled? (checking assert isn't the same, as debug should eventually drive other features)
  2. assertions will now be off by default – this is a change!
  3. is the addition of the name local-bindings to clojure.core cool?


 Comments   
Comment by Assembla Importer [ 24/Aug/10 6:05 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/250
Attachments:
add-clojure-debug-flag.patch - https://www.assembla.com/spaces/clojure/documents/aUWn50c64r35E-eJe5aVNr/download/aUWn50c64r35E-eJe5aVNr
assert-report-locals.patch - https://www.assembla.com/spaces/clojure/documents/aUWqLSc64r35E-eJe5aVNr/download/aUWqLSc64r35E-eJe5aVNr

Comment by Stuart Halloway [ 07/Dec/10 8:23 PM ]

Ignore the old patches. Considering the following implementation, please review and then move ticket to waiting on Stu:

  1. RT will check system property "clojure.debug", default to false
  2. property will set the root binding for the current *assert*, plus a new *debug* flag. (Debug builds can and will drive other things than just asserts.)
  3. does Compile.java need to push *assert* or *debug* as thread local bindings, or can they be root-bound only when compiling clojure?
  4. will add *debug* binding to clojure.main/with-bindings. Anywhere else?
  5. build.xml should not have to change – system properties will flow through (and build.xml may not be around much longer anyway)
  6. once we agree on the approach, I will ping maven plugin and lein owners so that they flow the setting through
  7. better assertion messages will be a separate ticket
  8. what is the interaction between *debug* and *unchecked-math*? Change checks to (and *unchecked-math* (not *debug*))}?
Comment by Rich Hickey [ 08/Dec/10 11:00 AM ]

#3 - root bound only
#4 - should not be in with-bindings for same reason as #3 - we don't want people to set! *debug* nor *assert*
#8 - yes, wrapping that in a helper fn

#6 - my biggest reservation is that this isn't yet informed by maven best practices

Comment by Stuart Sierra [ 08/Dec/10 2:09 PM ]

System properties can be passed through Maven, so I do not anticipate this being a problem.

However, I would prefer *assert* to remain true by default.

Comment by Chas Emerick [ 09/Dec/10 7:19 AM ]

SS is correct about this approach not posing any issue for Maven. In addition, the build could easily be set up to always emit two jars, one "normal", one "debug".

I'd suggest that, while clojure.debug might have broad effect, additional properties should be available to provide fine-grained control over each of the additional "debug"-related parameterizations that might become available in the future.


I'd like to raise a couple of potentially tangential concerns (after now thinking about assertions for a bit in the above context), some or all of which may simply be a result of my lack of understanding in various areas.

Looking at where assert is used in core.clj (only two places AFAICT: validating arguments to derive and checking pre- and post-conditions in fn), it would seem unwise to make it false by default. i.e. non-Named values would be able to get into hierarchies, and pre- and post-conditions would simply be ignored.

It's my understanding that assertions (talking here of the JVM construct, from which Clojure reuses AssertionError) should not be used to validate arguments to public API functions, or used to validate any aspect of a function's normal operation (i.e. "where not to use assertions"). That would imply that derive should throw IllegalArugmentException when necessary, and fn pre- and post-conditions should perhaps throw IllegalStateException – or, in any case, something other than AssertionError via assert. This would match up far better with most functions in core using assert-args rather than assert, the former throwing IllegalArgumentException rather than AssertionError.

That leads me to the question: is assert (and *assert*) intended to be a Clojure construct, or a quasi-interop form?

If the former, then it can roughly have whatever semantics we want, but then it seems like it should not be throwing AssertionError.

If the latter, then AssertionError is appropriate on the JVM, but then we need to take care that assertions can be enabled and disabled at runtime (without having to switch around different builds of Clojure), ideally using the host-defined switches (e.g. -ea and friends) and likely not anything like *assert*. I don't know if this is possible or practical at this point (I presume this would require nontrivial compiler changes).


Hopefully the above is not water under the bridge at this point. Thank you in advance for your forbearance.

Comment by Rich Hickey [ 09/Dec/10 8:08 AM ]

Thanks for the useful input Chas. Nothing is concluded yet. I think we should step back and look at the objective here, before moving forward with a solution. Being a dynamic language, there are many things we might want to validate about our programs, where the cost of checking is something we are unwilling to pay in production.

Being a macro, assert has the nice property that, should *assert* not be true during compilation, it generates nil, no conditional test at all. Thus right now it is more like static conditional compilation.

Java assert does have runtime toggle-ability, via -ea as you say. I haven't looked at the bytecode generated for Java asserts, but it might be possible for Clojure assert to do something similar, if the runtime overhead is negligible. It is quite likely that HotSpot has special code for eliding the assertion bytecode given a single check of some flag. I'm just not sure that flag is Class.desiredAssertionStatus.

Whether this turns into changes in assert or pre/post conditions, best practices etc is orthogonal and derived. Currently we don't have a facility to run without the checks. We need to choose between making them disappear during compilation (debug build) or runtime (track -ea) or both. Then we can look at how that is reflected in assert/pre-post and re-examine existing use of both. The "where not to use assertions" doc deals with them categorically, but in not considering their cost, seems unrealistic IMO.

I'd appreciate it if someone could look into how assert is generated and optimized by Java itself.

Comment by Chas Emerick [ 09/Dec/10 5:04 PM ]

Bytecode issues continue to be above my pay grade, unfortunately…

A few additional thoughts in response that you may or may not be juggling already:

assert being a macro makes total sense for what it's doing. Trouble is, "compile-time" is a tricky concept in Clojure: there's code-loading-time, AOT-compile-time, and transitively-AOT-compile-time. Given that, it's entirely possible for an application deployed to a production environment that contains a patchwork of code or no code produced by assert usages in various libraries and namespaces depending upon when those libs and ns' were loaded, AOT-compiled, or their dependents AOT-compiled, and the value of *assert* at each of those times. Of course, this is the case for all such macros whose results are dependent upon context-dependent state (I think this was a big issue with clojure.contrib.logging, making it only usable with log4j for a while).

What's really attractive about the JVM assertion mechanism is that it can be parameterized for a given runtime on a per-package basis, if desired. Reimplementing that concept so that assert can be *ns*-sensitive seems like it'd be straightforward, but the compile-time complexity already mentioned remains, and the idea of having two independently-controlled assertion facilities doesn't sound fun.

I know nearly nothing about the CLR, but it would appear that it doesn't provide for anything like runtime-controllable assertions.

Comment by Stuart Halloway [ 29/Dec/10 3:17 PM ]

The best (dated) evidence I could find says that the compiler sets a special class static final field $assertionsDisabled based on the return of desiredAssertionStatus. HotSpot doesn't do anything special with this, dead code elimination simply makes it go away. The code indeed compiles this way:

11: getstatic #6; //Field $assertionsDisabled:Z
14: ifne 33
17: lload_1
18: lconst_0
19: lcmp
20: ifeq 33
23: new #7; //class java/lang/AssertionError
26: dup
27: ldc #8; //String X should be zero
29: invokespecial #9; //Method java/lang/AssertionError."<init>":(Ljava/lang/Object;)V
32: athrow

Even if we were 100% sure that assertion removal was total, I would still vote for a separate Clojure-level switch, for the following reasons:

  1. I have a real and pressing need to disable some assertions, and I don't need the Java interop at all. Arguably others will be in the same boat.
  2. there will be multiple debugging facilities over time, and having a top-level debug switch is convenient for Clojure users.
  3. Java dis/enabling via command line flags is still possible as a separate feature. We could add this later as a (small) breaking change to our assert, or have a separate java-assert interop form. I am on the fence about which way to go here.
  4. I believe it is perfectly fine to throw an AssertionError from a non-Java-assertion-form. We don't believe in a world of a static exception hierarchy, and an assertion in production is a critical failure no matter what you call it. Even Scala does it http://daily-scala.blogspot.com/2010/03/assert-require-assume.html

Rich: awaiting your blessing to move forward on this.

Comment by Rich Hickey [ 07/Jan/11 9:42 AM ]

The compiler sets $assertionsDisabled when, in static init code? Is there special support in the classloader for it? Is there a link to the best dated evidence you found?

Comment by Stuart Halloway [ 07/Jan/11 9:51 AM ]
  1. Yes, in static init code
  2. There is no special support in the classloader, per Brian Goetz (private correspondence) last week. But dead code elimination is great: "The run-time cost of disabled assertions should indeed be zero for compiled code"
Comment by Stuart Halloway [ 07/Jan/11 9:57 AM ]

Link: Google "java assert shirazi". (Not posting link because I can't tell in 10 secs whether it includes my session information.)

Comment by Alexander Kiel [ 14/Mar/13 1:28 PM ]

Is there anything new on this issue? I also look for a convenient way to disable assertions in production.

Comment by Michał Łopuszyński [ 11/Oct/13 4:21 AM ]

I am also interested in any news on this issue.
Convenient way to enable/disable assertions at runtime (preferably via -ea/-da options) would be a great feature!

Comment by Michał Łopuszyński [ 23/Oct/13 3:12 AM ]

Btw. there is a library for runtime-toggable assertions available via clojars
https://github.com/pjstadig/assertions
This helped me a great deal.





[CLJ-274] cannot close over mutable fields (in deftype) Created: 23/Feb/10  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: deftype

Approval: Vetted

 Description   

Simplest case:

user=>
(deftype Bench [#^{:unsynchronized-mutable true} val]
Runnable
(run [_]
(fn [] (set! val 5))))

java.lang.IllegalArgumentException: Cannot assign to non-mutable: val (NO_SOURCE_FILE:5)

Functions should be able to mutate mutable fields in their surrounding deftype (just like inner classes do in Java).

Filed as bug, because the loop special form expands into a fn form sometimes:

user=>
(deftype Bench [#^{:unsynchronized-mutable true} val]
Runnable
(run [_]
(let [x (loop [] (set! val 5))])))
java.lang.IllegalArgumentException: Cannot assign to non-mutable: val (NO_SOURCE_FILE:9)



 Comments   
Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

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

Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

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

Now gives exception listed.

Comment by Assembla Importer [ 01/Oct/10 9:35 AM ]

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





[CLJ-346] (pprint-newline :fill) is not handled correctly Created: 12/May/10  Updated: 03/Sep/13

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

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Tom Faulhaber
Resolution: Unresolved Votes: 0
Labels: print

Approval: Vetted
Waiting On: Tom Faulhaber

 Description   

Filled pretty printing (where we try to fit as many elements on a line as possible) is being too aggressive as we can see when we try to print the following array:

user> (binding [*print-right-margin* 20] (pprint (int-array (range 10))))

Produces:

[0,
1,
2,
3,
4, 5, 6, 7, 8, 9]

Rather than

[0, 1, 2, 3, 4,
5, 6, 7, 8, 9]

or something like that. (I haven't worked through the exact correct representation for this case).

We currently only use :fill style newlines for native java arrays.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/346
Attachments:
0347-pprint-update-2.diff - https://www.assembla.com/spaces/clojure/documents/diLxv6y4Sr35GVeJe5cbLr/download/diLxv6y4Sr35GVeJe5cbLr

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

stu said: [file:diLxv6y4Sr35GVeJe5cbLr]

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

stu said: The second patch includes the first, and adds another test.

Comment by Assembla Importer [ 24/Aug/10 8:01 AM ]

tomfaulhaber said: This patch was attached to the wrong bug. It should be attached to bug #347. There is no fix for this bug yet.

Comment by Rich Hickey [ 05/Nov/10 8:07 AM ]

Is this current?

Comment by Stuart Halloway [ 29/Nov/10 8:48 PM ]

Tom, this patch doesn't apply, and I am not sure why. Can you take a look?





[CLJ-415] smarter assert (prints locals) Created: 29/Jul/10  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: errormsgs

Attachments: Text File clj-415-assert-prints-locals-v1.txt    
Approval: Vetted
Waiting On: Rich Hickey

 Description   

Here is an implementation you can paste into a repl. Feedback wanted:

(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
  "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")]
           (throw (AssertionError. (apply str "Assert failed: " (pr-str '~x) sep#
                                          (map (fn [[k# v#]] (str "\t" k# " : " v# sep#)) ~bindings)))))))))


 Comments   
Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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
Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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).

Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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

Comment by Jeff Weiss [ 15/Dec/10 1:33 PM ]

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)))))))))
Comment by Stuart Halloway [ 04/Jan/11 8:31 PM ]

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.

Comment by Stuart Halloway [ 28/Jan/11 1:14 PM ]

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))))))))))
Comment by Jeff Weiss [ 27/May/11 8:16 AM ]

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))))
Comment by Andy Fingerhut [ 18/Nov/12 1:06 AM ]

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.





[CLJ-771] Move unchecked-prim casts to clojure.unchecked Created: 07/Apr/11  Updated: 03/Sep/13

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

Type: Enhancement Priority: Minor
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Unresolved Votes: 0
Labels: math

Attachments: Text File clj-771-move-unchecked-casts-patch-v5.txt     Text File move-unchecked-casts.patch     Text File move-unchecked-casts-v2.patch    
Patch: Code and Test
Approval: Vetted
Waiting On: Rich Hickey

 Description   

Per Rich's comment in CLJ-767:

Moving unchecked coercions into unchecked ns is ok



 Comments   
Comment by Alexander Taggart [ 29/Apr/11 3:41 PM ]

Requires that patch on CLJ-782 be applied first.

Comment by Stuart Sierra [ 31/May/11 10:43 AM ]

Applies on master as of commit 66a88de9408e93cf2b0d73382e662624a54c6c86

Comment by Rich Hickey [ 09/Dec/11 8:40 AM ]

still considering when to incorporate this

Comment by John Szakmeister [ 19/May/12 9:36 AM ]

v2 of the patch applies to master as of commit eccde24c7fb63679f00c64b3c70c03956f0ce2c3

Comment by Andy Fingerhut [ 07/Sep/12 12:40 AM ]

Patch clj-771-move-unchecked-casts-patch-v3.txt dated Sep 6 2012 is the same as Alexander Taggart's patch move-unchecked-casts.patch except that it has been updated to apply cleanly to latest Clojure master.

Comment by Andy Fingerhut [ 20/Oct/12 12:18 PM ]

Patch clj-771-move-unchecked-casts-patch-v4.txt dated Oct 20 2012 is the same as Alexander Taggart's patch move-unchecked-casts.patch except that it has been updated to apply cleanly to latest Clojure master.

Comment by Andy Fingerhut [ 01/Jan/13 11:37 AM ]

The patch clj-771-move-unchecked-casts-patch-v4.txt applies cleanly to latest master and passes all tests. Rich marked this ticket as Incomplete on Dec 9 2011 with the comment "still considering when to incorporate this" above. Is it reasonable to change it back to Vetted or Screened so it can be considered again, perhaps after Release 1.5 is made?

Comment by Andy Fingerhut [ 13/Feb/13 12:50 AM ]

Patch clj-771-move-unchecked-casts-patch-v5.txt dated Feb 12 2013 is the same as Alexander Taggart's patch move-unchecked-casts.patch except that it has been updated to apply cleanly to latest Clojure master.





[CLJ-47] GC Issue 43: Dead code in generated bytecode Created: 17/Jun/09  Updated: 26/Aug/13

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

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   
Reported by Levente.Santha, Jan 11, 2009
The bug was described in detail in this thread: http://groups.google.com/
group/clojure/browse_thread/thread/81ba15d7e9130441

For clojure.core$last__2954.invoke the correct bytecode would be (notice 
the removed "goto    65" after "41:  goto    0"):

public java.lang.Object invoke(java.lang.Object)   throws 
java.lang.Exception;
  Code:
   0:   getstatic       #22; //Field const__0:Lclojure/lang/Var;
   3:   invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   6:   checkcast       #39; //class clojure/lang/IFn
   9:   aload_1
   10:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   15:  dup
   16:  ifnull  44
   19:  getstatic       #47; //Field java/lang/Boolean.FALSE:Ljava/lang/
Boolean;
   22:  if_acmpeq       45
   25:  getstatic       #22; //Field const__0:Lclojure/lang/Var;
   28:  invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   31:  checkcast       #39; //class clojure/lang/IFn
   34:  aload_1
   35:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   40:  astore_1
   41:  goto    0
   44:  pop
   45:  getstatic       #26; //Field const__1:Lclojure/lang/Var;
   48:  invokevirtual   #37; //Method clojure/lang/Var.get:()Ljava/lang/
Object;
   51:  checkcast       #39; //class clojure/lang/IFn
   54:  aload_1
   55:  aconst_null
   56:  astore_1
   57:  invokeinterface #41,  2; //InterfaceMethod clojure/lang/IFn.invoke:
(Ljava/lang/Object;)Ljava/lang/Object;
   62:  areturn

Our JIT reported incorrect stack size along the basic block introduced by 
the unneeded goto.
The bug was present in SVN rev 1205.


 Comments   
Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

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

Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

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

Comment by Assembla Importer [ 08/Oct/10 10:21 AM ]

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.





[CLJ-84] GC Issue 81: compile gen-class fail when class returns self Created: 17/Jun/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Backlog

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Rich Hickey
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted
Waiting On: Chouser

 Description   
Reported by davidhaub, Feb 14, 2009

When attempting to compile the following program, clojure fails with a
ClassNotFoundException.  It occurs because one of the methods returns the
same class that is being generated.  If the returnMe method below is
changed to return an Object, the compile succeeds.

Beware when testing! If the classpath contains a class file (say from a
prior successful build when the returnMe method was changed to return an
object), the compile will succeed.  Always clear out the
clojure.compile.path prior to compiling.

;badgenclass.clj
(ns badgenclass
  (:gen-class
     :state state
     :methods
     [[returnMe [] badgenclass]]
     :init init))
(defn -init []
  [[] nil])

(defn -returnMe [this]
  this)

#!/bin/sh
rm -rf classes
mkdir classes
java -cp lib/clojure.jar:classes:. -Dclojure.compile.path=classes \
clojure.lang.Compile badgenclass


Comment 1 by chouser, Mar 07, 2009

Attached is a patch that accepts strings or symbols for parameter and return class
names, and generates the appropriate bytecode without calling Class/forName.  It
fixes this issue, but because 'ns' doesn't resolve :gen-class's arguments, class
names aren't checked as early anymore.  :gen-class-created classes with invalid
parameter or return types can even be instantiated, and no error will be reported
until the broken method is called.

One possible alternative would be to call Class/forName on any symbols given, but
allow strings to use the method given by this patch.  To return your own type, you'd
need a method defined like:

  [returnMe [] "badgenclass"]

Any thoughts?


 Comments   
Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/84
Attachments:
genclass-allow-unresolved-classname.patch - https://www.assembla.com/spaces/clojure/documents/cWS6Aww30r3RbzeJe5afGb/download/cWS6Aww30r3RbzeJe5afGb

Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

oranenj said: [file:cWS6Aww30r3RbzeJe5afGb]: on comment 1

Comment by Assembla Importer [ 28/Sep/10 5:09 PM ]

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

Comment by Stuart Halloway [ 30/Oct/10 11:58 AM ]

The approach take in the initial patch (delaying resolution of symbols into classes) is fine: gen-class makes no promise about when this happens, and the dynamic approach feels more consistent with Clojure. I think the proposed (but not implemented) use of string/symbol to control when class names are resolved is a bad idea: magical and not implied by the types.

Needed:

  • update the patch to apply cleanly
  • consider whether totype could live in clojure.reflect.java. (Beware load order dependencies.)
Comment by Chouser [ 30/Oct/10 9:29 PM ]

Wow, 18-month-old patch, back to haunt me for Halloway'een

So what does it mean that the assignee is Rich, but it's waiting on me?

Comment by Stuart Halloway [ 01/Nov/10 9:17 AM ]

I am using Approval = Incomplete plus Waiting On = Someone to let submitters know that there is feedback waiting, and that they can move the ticket forward by acting on it. The distinction is opportunity to contribute (something has been provided to let you move forward) vs. expectation of contribution.





[CLJ-291] (take-nth 0 coll) redux... Created: 08/Apr/10  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

I dont seem to be able make the old ticket uninvalid so here goes
(take-nth 0 coll) causes (at least on Solaris) infinite space and time consumption
It's not a printout error as the following code causes the problem too

(let [j 0
firstprod (apply * (doall (map #(- 1 %) (take-nth j (:props mix)))))]) ; from my parameter update function

I used jvisualvm and the jvm is doing some RNI call - no clojure code is running at all
If left alone it will crash the jvm with all heap space consumed
0 is an InvalidArgument for take-nth
I wouldnt mind if it produced an infinite lazy sequence of nils even though thats wrong
It doesnt do this though it actively destroys the JVM
Its a bug nasty destructive and it took me half a day to figure out what was going on
please let someone fix it!



 Comments   
Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/291
Attachments:
fixbug291.diff - https://www.assembla.com/spaces/clojure/documents/dfNhoS2Cir3543eJe5cbLA/download/dfNhoS2Cir3543eJe5cbLA

Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

bhurt said: Before this bug gets marked as invalid as well, let me point out that the problem here is that (take-nth 0 any-list) is a meaningless construction- the only question is what to do when this happens. IMHO, the correct behavior is to throw an exception.

Comment by Assembla Importer [ 17/Oct/10 9:47 AM ]

ataggart said: [file:dfNhoS2Cir3543eJe5cbLA]: throws IllegalArgumentException on negative step size

Comment by Stuart Halloway [ 29/Oct/10 10:36 AM ]

Does calling (take-nth 0 ...) cause the problem, or only realizing the result?

Comment by Chouser [ 29/Oct/10 11:06 AM ]

I'm not seeing a problem. Calling take-nth and even partially consuming the seq it returns works fine for me:

(take 5 (take-nth 0 [1 2 3]))
;=> (1 1 1 1 1)

Note however that it is returning an infinite lazy seq. The example in the issue description seems to include essentially (doall <infinite-lazy-seq>) which does blow the heap:

(doall (range))

This issue still strikes me as invalid.

Comment by Rich Hickey [ 29/Oct/10 11:06 AM ]

(take-nth 0 ...) returns an infinite sequence of the first item:

(take 12 (take-nth 0 [1 2 3]))
=> (1 1 1 1 1 1 1 1 1 1 1 1)

Is something other than this happening on Solaris?





[CLJ-69] GC Issue 66: Add "keyset" to Clojure; make .keySet for APersistentMap return IPersistentSet Created: 17/Jun/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: Backlog
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   
Reported by wolfe.a.jason, Feb 04, 2009

Describe the feature/change.

Add "keyset" to Clojure; make .keySet for APersistentMap return an
IPersistentSet

Was this discussed on the group? If so, please provide a link to the
discussion:

http://groups.google.com/group/clojure/browse_thread/thread/66e708e477ae992f/ff3d8d588068b60e?hl=en#ff3d8d588068b60e

-----------------------------------------------------

A patch is attached.  Some notes:

I chose to add a "keyset" function, rather than change the existing "keys",
so as to avoid breaking anything.

The corresponding RT.keyset function just calls .keySet on the argument.
I would have liked to have "keyset" return an IPersistentSet even when
passed a (non-Clojure) java.util.Map, but this seems impossible to do in
sublinear time because of essentially the same limitation mentioned in the
above thread (the Map interface does not support getKey() or entryAt()) --
assuming, again, that "get" is supposed to return the actual (identical?)
key in a set, and not just an .equal key.

I then changed the implementation of .keySet for APersistentMap to
essentially copy APersistentSet.  A more concise alternative would have
been to extend APersistentSet and override the .get method, but that made
me a bit nervous (since if APeristentSet changed this could break). 

Anyway, this is my first patch for the Java side of Clojure, and I'm not
yet solid on the conventions and aesthetics, so
comments/questions/criticisms/requests for revisions are very welcome.


 Comments   
Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/69
Attachments:
keyset.patch - https://www.assembla.com/spaces/clojure/documents/dKgE6mw3Gr3O2PeJe5afGb/download/dKgE6mw3Gr3O2PeJe5afGb

Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

oranenj said: [file:dKgE6mw3Gr3O2PeJe5afGb]

Comment by Assembla Importer [ 28/Sep/10 7:00 AM ]

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

Comment by Stuart Halloway [ 03/Dec/10 1:12 PM ]

patch not in correct format





[CLJ-211] Support arbitrary functional destructuring via -> and ->> Created: 17/Nov/09  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

Support arbitrary functional destructuring, that is the use of
any function in any destructuring form to help unpack data in
arbitrary ways.

The discussion began here:
http://clojure-log.n01se.net/date/2009-11-17.html#09:31c

The attached patch implements the spec described here:
http://clojure-log.n01se.net/date/2009-11-17.html#10:50a

That is, the following examples would now work:

user=> (let [(-> str a) 1] a)
"1"

user=> (let [[a (-> str b) c] [1 2]] (list a b c))
(1 "2" nil)

user=> (let [(->> (map int) [a b]) "ab"] (list a b))
(97 98)



 Comments   
Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/211
Attachments:
destructuring-fns.diff - https://www.assembla.com/spaces/clojure/documents/aHWQ_W06Kr3O89eJe5afGb/download/aHWQ_W06Kr3O89eJe5afGb

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

chouser@n01se.net said: [file:aHWQ_W06Kr3O89eJe5afGb]: [PATCH] Support -> and ->> in destructuring forms.

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

cgrand said: I think the current patch suffers from the problem described here http://groups.google.com/group/clojure-dev/msg/80ba7fad2ff04708 too.

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

richhickey said: so, don't use syntax-quote, just use clojure.core/->

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

chouser@n01se.net said: Only -> and ->> are actually legal here anyway – if you've locally bound foo to -> there's not really any good reason to think (fn [(foo inc a)] a) should work. And if you've redefined -> or ->> to mean something else in your ns, do we need to catch that at compile time, or is it okay to emit the rearranged code and see what happens?

In short, would '#{> ->> clojure.core/> clojure.core/->>} be sufficient?

Comment by Assembla Importer [ 28/Sep/10 6:57 AM ]

cgrand said: Only -> and ->> are legal here but what if they are aliased or shadowed? Instead of testing the symboil per se I would check, if:

  • the symbol is not in &env
  • the symbol resolve to #'clojure.core/> or #'clojure.core/>>
(when-not (&env (first b)) (#{#'clojure.core/-> #'clojure.core/->>} (resolve (first b))))

but it requires to change destructure's sig to pass the env around

Comment by Stuart Halloway [ 03/Dec/10 1:03 PM ]

Rich: Are you assigned to this by accident? If so, please deassign yourself.





[CLJ-445] Method/Constructor resolution does not factor in widening conversion of primitive args Created: 29/Sep/10  Updated: 27/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Major
Reporter: Alexander Taggart Assignee: Alexander Taggart
Resolution: Unresolved Votes: 3
Labels: None

Attachments: Text File clj-445-prim-conversion-update-2-patch.txt     Text File prim-conversion.patch     Text File prim-conversion-update-1.patch     Text File reorg-reflector.patch    
Approval: Vetted

 Description   

Problem:
When making java calls (or inlined functions), if both args and param are primitive, no widening conversion is used to locate the proper overloaded method/constructor.

Examples:

user=> (Integer. (byte 0))
java.lang.IllegalArgumentException: No matching ctor found for class java.lang.Integer (NO_SOURCE_FILE:0)
</code></pre>
The above occurs because there is no Integer(byte) constructor, though it should match on Integer(int).
<pre><code>user=> (bit-shift-left (byte 1) 1)
Reflection warning, NO_SOURCE_PATH:3 - call to shiftLeft can't be resolved.
2

In the above, a call is made via reflection to Numbers.shiftLeft(Object, Object) and its associated auto-boxing, instead of directly to the perfectly adequate Numbers.shiftLeft(long, int).

Workarounds:
Explicitly casting to the formal type.

Ancillary benefits of fixing:
It would also reduce the amount of method overloading, e.g., RT.intCast(char), intCast(byte), intCast(short), could all be removed, since such calls would pass to RT.intCast(int).



 Comments   
Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/445
Attachments:
fixbug445.diff - https://www.assembla.com/spaces/clojure/documents/b6gDSUZOur36b9eJe5cbCb/download/b6gDSUZOur36b9eJe5cbCb

Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

ataggart said: [file:b6gDSUZOur36b9eJe5cbCb]

Comment by Assembla Importer [ 23/Oct/10 6:43 PM ]

ataggart said: Also fixes #446.

Comment by Stuart Halloway [ 03/Dec/10 12:50 PM ]

The patch is causing a test failure

[java] Exception in thread "main" java.lang.IllegalArgumentException: 
     More than one matching method found: equiv, compiling:(clojure/pprint/cl_format.clj:428)

Can you take a look?

Comment by Stuart Halloway [ 28/Jan/11 12:30 PM ]

The failing test happens when trying to find the correct equiv for signature (Number, long). Is the compiler wrong to propose this signature, or is the resolution method wrong in not having an answer? (It thinks two signatures are tied: (Object, long) and (Number, Number).)

Exception in thread "main" java.lang.IllegalArgumentException: More than one matching method found: equiv, compiling:(clojure/pprint/cl_format.clj:428)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6062)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6050)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.access$100(Compiler.java:35)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5492)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2372)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3277)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6057)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2385)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$IfExpr$Parser.parse(Compiler.java:2385)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5527)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5231)
	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:4667)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3397)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6053)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6043)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.access$100(Compiler.java:35)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:480)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	at clojure.lang.Compiler.analyze(Compiler.java:5866)
	at clojure.lang.Compiler.analyze(Compiler.java:5827)
	at clojure.lang.Compiler.eval(Compiler.java:6114)
	at clojure.lang.Compiler.load(Compiler.java:6545)
	at clojure.lang.RT.loadResourceScript(RT.java:340)
	at clojure.lang.RT.loadResourceScript(RT.java:331)
	at clojure.lang.RT.load(RT.java:409)
	at clojure.lang.RT.load(RT.java:381)
	at clojure.core$load$fn__1427.invoke(core.clj:5308)
	at clojure.core$load.doInvoke(core.clj:5307)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.pprint$eval3969.invoke(pprint.clj:46)
	at clojure.lang.Compiler.eval(Compiler.java:6110)
	at clojure.lang.Compiler.load(Compiler.java:6545)
	at clojure.lang.RT.loadResourceScript(RT.java:340)
	at clojure.lang.RT.loadResourceScript(RT.java:331)
	at clojure.lang.RT.load(RT.java:409)
	at clojure.lang.RT.load(RT.java:381)
	at clojure.core$load$fn__1427.invoke(core.clj:5308)
	at clojure.core$load.doInvoke(core.clj:5307)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.core$load_one.invoke(core.clj:5132)
	at clojure.core$load_lib.doInvoke(core.clj:5169)
	at clojure.lang.RestFn.applyTo(RestFn.java:143)
	at sun.reflect.GeneratedMethodAccessor11.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:77)
	at clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:28)
	at clojure.core$apply.invoke(core.clj:602)
	at clojure.core$load_libs.doInvoke(core.clj:5203)
	at clojure.lang.RestFn.applyTo(RestFn.java:138)
	at sun.reflect.GeneratedMethodAccessor11.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
	at java.lang.reflect.Method.invoke(Method.java:597)
	at clojure.lang.Reflector.invokeMatchingMethod(Reflector.java:77)
	at clojure.lang.Reflector.invokeInstanceMethod(Reflector.java:28)
	at clojure.core$apply.invoke(core.clj:604)
	at clojure.core$use.doInvoke(core.clj:5283)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.main$repl.doInvoke(main.clj:196)
	at clojure.lang.RestFn.invoke(RestFn.java:422)
	at clojure.main$repl_opt.invoke(main.clj:267)
	at clojure.main$main.doInvoke(main.clj:362)
	at clojure.lang.RestFn.invoke(RestFn.java:409)
	at clojure.lang.Var.invoke(Var.java:401)
	at clojure.lang.AFn.applyToHelper(AFn.java:163)
	at clojure.lang.Var.applyTo(Var.java:518)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: More than one matching method found: equiv
	at clojure.lang.Reflector.getMatchingParams(Reflector.java:639)
	at clojure.lang.Reflector.getMatchingParams(Reflector.java:578)
	at clojure.lang.Reflector.getMatchingMethod(Reflector.java:569)
	at clojure.lang.Compiler$StaticMethodExpr.<init>(Compiler.java:1439)
	at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:896)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6055)
	... 115 more
Comment by Alexander Taggart [ 08/Feb/11 6:27 PM ]

In working on implementing support for vararg methods, I found a number of flaws with the previous solutions. Please disregard them.

I've attached a single patch (reflector-compiler-numbers.diff) which is a rather substantial overhaul of the Reflector code, with some enhancements to the Compiler and Numbers code.

The patch notes:

  • Moved reflection functionality from Compiler to Reflector.
  • Reflector supports finding overloaded methods by widening conversion, boxing conversion, and casting.
  • During compilation Reflector attempts to find best wildcard match.
  • Reflector refers to *unchecked-math* when reflectively invoking methods and constructors.
  • Both Reflector and Compiler support variable arity java methods and constructor; backwards compatible with passing an array or nil in the vararg slot.
  • Added more informative error messages to Reflector.
  • Added tests to clojure.test-clojure.reflector.
  • Altered overloaded functions of clojure.lang.Numbers to service Object/double/long params; fixes some ambiguity issues and avoids unnecessary boxing in some cases.
  • Patch closes issues 380, 440, 445, 666, and possibly 259 (not enough detail provided).
Comment by Alexander Taggart [ 10/Feb/11 7:35 PM ]

Updated patch to fix a bug where a concrete class with multiple identical Methods (e.g., one from an interface, another from an abstract class) would result in ambiguity. Now resolved by arbitrary selection (this is what the original code did as well albeit not explicitly).

Comment by Alexander Taggart [ 25/Feb/11 9:29 PM ]

Updated patch to work with latest master branch.

Comment by Stuart Halloway [ 06/Mar/11 1:54 PM ]

patch appears to be missing test file clojure/test_clojure/reflector.clj.

Comment by Alexander Taggart [ 06/Mar/11 2:39 PM ]

Bit by git.

Patch corrected to contain clojure.test-clojure.reflector.

Comment by Stuart Halloway [ 11/Mar/11 10:30 AM ]

Rich: I verified that the patch applied but reviewed only briefly, knowing you will want to go over this one closely.

Comment by Stuart Halloway [ 11/Mar/11 10:55 AM ]

After applying this patch, I am getting method missing errors:

java.lang.NoSuchMethodError: clojure.lang.Numbers.lt(JLjava/lang/Object;)

but only when using compiled code, e.g. the same code works in the REPL and then fails after compilation. Haven't been able to isolate an example that I can share here yet, but hoping this will cause someone to have an "a, hah" moment...

Comment by Alexander Taggart [ 02/Apr/11 12:55 PM ]

The patch now contains only the minimal changes needed to support widening conversion. Cleanup of Numbers overloads, etc., can wait until this patch gets applied. The vararg support is in a separate patch on CLJ-440.

Comment by Christopher Redinger [ 15/Apr/11 12:50 PM ]

Please test patch

Comment by Christopher Redinger [ 21/Apr/11 11:02 AM ]

FYI: Patch applies cleanly on master and all tests pass as of 4/21 (2011)

Comment by Christopher Redinger [ 22/Apr/11 9:57 AM ]

This work is too big to take into the 1.3 beta right now. We'll revisit for a future release.

Comment by Alexander Taggart [ 28/Apr/11 1:19 PM ]

To better facilitate understanding of the changes, I've broken them up into two patches, each with a number of isolable, incremental commits:

reorg-reflector.patch: Moves the reflection/invocation code from Compiler to Reflector, and eliminates redundant code. The result is a single code base for resolving methods/constructors, which will allow for altering that mechanism without excess external coordination. This contains no behaviour changes.

prim-conversion.patch: Depends on the above. Alters the method/constructor resolution process:

  • more consistent with java resolution, especially when calling pre-1.5 APIs
  • adds support for widening conversion of primitive numerics of the same category (this is more strict than java, and more clojuresque)
  • adds support for wildcard matches at compile-time (i.e., you don't need to type-hint every arg to avoid reflection).

This also provides a base to add further features, e.g., CLJ-666.

Comment by Alexander Taggart [ 29/Apr/11 3:01 PM ]

It's documented in situ, but here are the conversion rules that the reflector uses to find methods:

  1. By Type:
    • object to ancestor type
    • primitive to a wider primitive of the same numeric category (new)
  2. Boxing:
    • boxed number to its primitive
    • boxed number to a wider primitive of the same numeric category (new for Short and Byte args)
    • primitive to its boxed value
    • primitive to Number or Object (new)
  3. Casting:
    • long to int
    • double to float
Comment by Alexander Taggart [ 10/May/11 3:13 PM ]

prim-conversion-update-1.patch applies to current master.

Comment by Alexander Taggart [ 11/May/11 2:15 PM ]

Created CLJ-792 for the reflector reorg.

Comment by Stuart Sierra [ 17/Feb/12 2:29 PM ]

prim-conversion-update-1.patch does not apply as of f5bcf64.

Is CLJ-792 now a prerequisite of this ticket?

Comment by Alexander Taggart [ 17/Feb/12 3:15 PM ]

Yes, after the original patch was deemed "too big".

After this much time with no action from TPTB, feel free to kill both tickets.

Comment by Andy Fingerhut [ 20/Feb/12 2:04 PM ]

Again, not sure if this is any help, but I've tested starting from Clojure head as of Feb 20, 2012, applying clj-792-reorg-reflector-patch2.txt attached to CLJ-792, and then applying clj-445-prim-conversion-update-2-patch.txt attached to this ticket, and the result compiles and passes all but 2 tests. I don't know whether those failures are easy to fix or not, or whether issues may have been introduced with these patches.





[CLJ-326] add :as-of option to refer Created: 30/Apr/10  Updated: 26/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

Type: Enhancement Priority: Minor
Reporter: Christophe Grand Assignee: Christophe Grand
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

Discussed here: http://groups.google.com/group/clojure-dev/msg/74af612909dcbe56

:as-of allows library authors to specify a known subset of vars to refer from clojure (or any other library which would use :added metadata).

(ns foo (:refer-clojure :as-of "1.1")) is equivalent to (ns foo (:refer-clojure :only [public-documented-vars-which-already-existed-in-1.1]))



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

Converted from http://www.assembla.com/spaces/clojure/tickets/326
Attachments:
add-as-of-to-refer.patch - https://www.assembla.com/spaces/clojure/documents/a8SumUvcOr37SmeJe5cbLA/download/a8SumUvcOr37SmeJe5cbLA

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

cgrand said: [file:a8SumUvcOr37SmeJe5cbLA]: requires application of #325

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

richhickey said: Do we still need this?





[CLJ-348] reify allows use of qualified name as method parameter Created: 13/May/10  Updated: 26/Jul/13

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

Type: Defect Priority: Minor
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Approval: Vetted

 Description   

This should complain about using a fully-qualified name as a parameter:

(defmacro lookup []
`(reify clojure.lang.ILookup
(valAt [_ key])))

Instead it simply ignores that parameter in the method body in favour of clojure.core/key.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/348
Attachments:
0001-Add-a-test-for-348-reify-shouldn-t-accept-qualified-.patch - https://www.assembla.com/spaces/clojure/documents/d2xUJIxTyr36fseJe5cbLA/download/d2xUJIxTyr36fseJe5cbLA

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

technomancy said: [file:d2xUJIxTyr36fseJe5cbLA]: A test to expose the unwanted behaviour

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

richhickey said: I'm not sure the bug is what you say it is, or the resolution should be what you suggest. The true problem is the resolution of key when qualified. Another possibility is to ignore the qualifier there.

Comment by Assembla Importer [ 24/Aug/10 8:03 AM ]

technomancy said: Interesting. So it's not appropriate to require auto-gensym here? Why are the rules different for reify methods vs proxy methods?

> (defmacro lookup []
`(proxy [clojure.lang.ILookup] []
(valAt [key] key)))
> (lookup)

Can't use qualified name as parameter: clojure.core/key
[Thrown class java.lang.Exception]





[CLJ-1104] Concurrent with-redefs do not unwind properly, leaving a var permanently changed Created: 07/Nov/12  Updated: 26/Jul/13

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

Type: Enhancement Priority: Minor
Reporter: Jason Wolfe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

Mac OS, Java 6


Attachments: Text File clj-1104-doc-unsafety-of-concurrent-with-redefs-v1.txt    
Patch: Code
Approval: Vetted

 Description   

On 1.4 and latest master:

user> (defn ten [] 10)
#'user/ten
user> (doall (pmap #(with-redefs [ten (fn [] %)] (ten)) (range 20 100)))
(20 21 22 23 24 25 34 27 28 29 30 31 32 33 34 35 36 37 38 39 39 35 42 43 44 45 48 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 79 87 88 89 90 91 92 93 94 95 97 92 98 99)
user> (ten)
79

Not sure if this is a bug per se, but the doc doesn't mention lack of concurrency safety and my expectation was that the original value would always be restored after any (arbitrarily interleaved) sequence of with-redefs calls.



 Comments   
Comment by Tim McCormack [ 07/Nov/12 8:50 PM ]

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

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

Comment by Herwig Hochleitner [ 08/Nov/12 9:17 AM ]

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

Comment by Stuart Halloway [ 25/Nov/12 6:41 PM ]

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

Comment by Andy Fingerhut [ 25/Nov/12 6:57 PM ]

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





[CLJ-731] Create macro to variadically unroll a combinator function definition Created: 26/Jan/11  Updated: 26/Jul/13

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

Type: Enhancement Priority: Minor
Reporter: Fogus Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Approval: Vetted

 Description   

Clojure contains a set of combinators that are implemented in a similar, but slightly different way. That is, they are implemented as a complete set of variadic overloads on both the call-side and also on the functions that they return. Visually, they all tend to look something like:

(defn foo
  ([f]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2 f3]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...)))
  ([f1 f2 f3 & fs]
     (fn
       ([] do stuff...)
       ([x] do stuff...)
       ([x y] do stuff...)
       ([x y z] do stuff...)
       ([x y z & args] do variadic stuff...))))

To build this type of function for each combinator is tedious and error-prone.

There must be a way to implement a macro that takes a "specification" of a combinator including:

1. name
2. docstring
3. do stuff template
4. do variadic stuff template

And builds something like the function foo above. This macro should be able to implement the current batch of combinators (assuming that http://dev.clojure.org/jira/browse/CLJ-730 is completed first for the sake of verification).



 Comments   
Comment by Stuart Halloway [ 28/Jan/11 9:03 AM ]

This seems useful. Rich, would you accept a patch?

Comment by Stuart Halloway [ 28/Jan/11 9:40 AM ]

Nevermind, just saw that Rich already suggested this on the dev list. Patch away.





Generated at Sun Nov 23 15:20:36 CST 2014 using JIRA 4.4#649-r158309.