<< Back to previous view

[CLJ-668] Improve slurp performance by using native Java StringWriter and jio/copy Created: 01/Nov/10  Updated: 06/Jan/15

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

Type: Enhancement Priority: Critical
Reporter: Jürgen Hötzel Assignee: Timothy Baldridge
Resolution: Unresolved Votes: 0
Labels: ft, io, performance

Attachments: File slurp-perf-patch.diff    
Patch: Code
Approval: Triaged

 Description   

Instead of copying each character from InputReader to StringBuffer.

Performance improvement:

Generate a 10meg file:
user> (spit "foo.txt" (apply str (repeat (* 1024 1024 10) "X")))

Test code:
user> (dotimes [x 100] (time (do (slurp "foo.txt") 0)))

From:
...
Elapsed time: 136.387 msecs"
"Elapsed time: 143.782 msecs"
"Elapsed time: 153.174 msecs"
"Elapsed time: 211.51 msecs"
"Elapsed time: 155.429 msecs"
"Elapsed time: 145.619 msecs"
"Elapsed time: 142.641 msecs"
...


To:
...
"Elapsed time: 23.408 msecs"
"Elapsed time: 25.876 msecs"
"Elapsed time: 41.449 msecs"
"Elapsed time: 28.292 msecs"
"Elapsed time: 25.765 msecs"
"Elapsed time: 24.339 msecs"
"Elapsed time: 32.047 msecs"
"Elapsed time: 23.372 msecs"
"Elapsed time: 24.365 msecs"
"Elapsed time: 26.265 msecs"
...

Approach: Use StringWriter and jio/copy vs character by character copy. Results from the current patch see a 4-5x perf boost after the jit warms up, with purely in-memory streams (ByteArrayInputStream over a 6MB string).

Patch: slurp-perf-patch.diff
Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 21/Apr/14 3:28 PM ]

This is double-better with the changes in Clojure 1.6 to improve jio/copy performance by using the NIO impl. Rough timing difference on a 25M file: old= 2316.021 msecs, new= 93.319 msecs.

Filer did not supply a patch and is not a contributor. If someone wants to make a patch (and better timing info demonstrating performance improvements), that would be great.

Comment by Timothy Baldridge [ 10/Sep/14 10:29 PM ]

Fixed the ticket formatting a bit, and added a patch I coded up tonight. Should be pretty close to the old patch, as we both use StringWriter, but I didn't really look at the old patch beyond noticing that it was using StringWriter.

Comment by Alex Miller [ 11/Sep/14 7:01 AM ]

Can you update the perf comparison on latest code and do both a small and big file?





[CLJ-1209] clojure.test does not print ex-info in error reports Created: 11/May/13  Updated: 14/May/15

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

Type: Enhancement Priority: Critical
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: clojure.test

Attachments: Text File 0002-CLJ-1209-show-ex-data-in-clojure-test.patch     File clj-test-print-ex-data.diff     Text File output-with-0002-patch.txt    
Patch: Code
Approval: Triaged

 Description   

clojure.test does not print the data attached to ExceptionInfo in error reports.

(use 'clojure.test)
(deftest ex-test (throw (ex-info "err" {:some :data})))
(ex-test)

ERROR in (ex-test) (core.clj:4591)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ExceptionInfo: err
 at clojure.core$ex_info.invoke (core.clj:4591)
    user/fn (NO_SOURCE_FILE:2)
    clojure.test$test_var$fn__7666.invoke (test.clj:704)
    clojure.test$test_var.invoke (test.clj:704)
    ...

Approach: In clojure.stacktrace, which clojure.test uses for printing exceptions, add a check for ex-data and pr it.

After:

ERROR in (ex-test) (core.clj:4591)
Uncaught exception, not in assertion.
expected: nil
  actual: clojure.lang.ExceptionInfo: err
{:some :data}
 at clojure.core$ex_info.invoke (core.clj:4591)
    user/fn (NO_SOURCE_FILE:3)
    clojure.test$test_var$fn__7667.invoke (test.clj:704)
    clojure.test$test_var.invoke (test.clj:704)

Patch: 0002-CLJ-1209-show-ex-data-in-clojure-test.patch



 Comments   
Comment by Alex Miller [ 20/Dec/13 9:53 AM ]

Great idea, thx for the patch!

Comment by Alex Miller [ 20/Dec/13 9:54 AM ]

Would be great to see a before and after example of the output.

Comment by Ivan Kozik [ 12/Jul/14 10:35 PM ]

Attaching sample output

Comment by Stuart Sierra [ 05/Sep/14 3:24 PM ]

As pointed out on IRC, there's a possible risk of trying to print an infinite lazy sequence that happened to be included in ex-data.

To mitigate, consider binding *print-length* and *print-level* to small numbers around the call to pr.

Comment by Stephen C. Gilardi [ 13/May/15 2:39 PM ]

http://dev.clojure.org/jira/browse/CLJ-1716 may cover this well enough that this issue can be closed.

Comment by Alex Miller [ 14/May/15 8:35 AM ]

I don't think 1716 covers it at all as clojure.test/clojure.stacktrace don't use the new throwable printing. But they could! And that might be a better solution than the patch here.

For example, the existing patch does not consider what to do about nested exceptions, some of which might have ex-data. The new printer handles all that in a consistent way.





[CLJ-1744] Clear unused locals Created: 03/Jun/15  Updated: 04/Jun/15

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

Type: Enhancement Priority: Critical
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: compiler, locals-clearing

Attachments: Text File 0001-CLJ-1744-clear-unused-locals.patch     Text File 0001-CLJ-1744-clear-unused-locals-v2.patch    
Patch: Code

 Description   

Clojure currently doesn't clear unused locals. This is problematic as some form of destructuring can generate unused/unusable locals that the compiler cannot clear and thus can cause head retention:

;; this works
user=> (loop [xs (repeatedly 2 #(byte-array (quot (.maxMemory (Runtime/getRuntime)) 10)))] (when (seq xs) (recur (rest xs))))
nil
;; this doesn't
user=>  (loop [[x & xs] (repeatedly 200 #(byte-array (quot (.maxMemory (Runtime/getRuntime)) 10)))] (when (seq xs) (recur xs)))
OutOfMemoryError Java heap space  clojure.lang.Numbers.byte_array (Numbers.java:1252)

Here's a macroexpansion that explicits this issue:

user=> (macroexpand-all '(loop [[a & b] c] [a b]))
(let* [G__21 c 
       vec__22 G__21
       a (clojure.core/nth vec__22 0 nil)
       b (clojure.core/nthnext vec__22 1)]
 (loop* [G__21 G__21]
   (let* [vec__23 G__21
          a (clojure.core/nth vec__23 0 nil)
          b (clojure.core/nthnext vec__23 1)]
     [a b])

The first two bindings of a and b will hold onto the head of c since they are never used and not accessible from the loop body they cannot be cleared.

The attached patch allows the compiler to clear unused locals making both examples work by trivially popping the binding value off the stack rather than assigning it to the local variable if analysis reveals it's never used.

Patch: 0001-CLJ-1744-clear-unused-locals-v2.patch



 Comments   
Comment by Michael Blume [ 03/Jun/15 12:57 PM ]

Nice =)





[CLJ-1458] Use transients in merge and merge-with Created: 04/Jul/14  Updated: 22/Jun/15

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

Type: Enhancement Priority: Critical
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: performance

Attachments: Text File 0001-very-simple-test-of-the-merge-function.patch     Text File clj-1458-4.patch     Text File CLJ-1458-transient-merge2.patch     Text File CLJ-1458-transient-merge3.patch     Text File CLJ-1458-transient-merge.patch     Text File merge-test-2.patch     File transient-merge.diff    
Patch: Code and Test
Approval: Triaged

 Description   

It would be nice if merge used transients.

Patches:

  • clj-1458-4.patch (code)
  • merge-test-2.patch (tests)

Move merge and merge-with later to leverage transduce. Leave older version as merge1 for use in cases prior to new merge and merge-with definition.

Screened by:



 Comments   
Comment by Jason Wolfe [ 13/Sep/14 5:09 PM ]

I will take a crack at a patch today.

Comment by Jason Wolfe [ 13/Sep/14 5:42 PM ]

This patch (transient-merge.diff) makes merge, merge-with, and zipmap (since it was right there and could obviously benefit from transients as well) use transients.

Three potential issues:

  • I had to move the functions, since they depend on transient and friends. I assume this is preferable to a forward declaration. This was the best place I could find, but happy to move them elsewhere.
  • I added multiple arities, to avoid potential performance cost of transient-ing a single argument. Happy to undo this if desired.
  • I had to slightly alter the logic in merge-with, since transient maps don't support contains? (or find).
Comment by Michał Marczyk [ 14/Sep/14 12:43 PM ]

I posted a separate ticket for zipmap, with patch, on 30/May/12: CLJ-1005.

Comment by Jason Wolfe [ 14/Sep/14 5:28 PM ]

Ah, sorry if I overstepped then. Happy to remove that change from this patch then if that will simplify things – just let me know.

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

alternate approach attached delaying merge until after protocols load, and then using transducers.

Comment by Michael Blume [ 28/Dec/14 11:50 PM ]

Looks like you're doing (get m k) twice – shouldn't that be thrown in a local?

Comment by Michael Blume [ 29/Dec/14 1:41 PM ]

um, put, in a local, I mean, 'throw' was a bad choice of word.

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

Yeah there's that – won't be using get anyways after CLJ-700 gets committed.

We should add performance tests too. merging two maps, three, many maps, also varying the sizes of the maps, and for merge-with, varying the % of collisions.

Need to go back to the (some identity) logic, otherwise metadata is propagated from maps other than the first provided. I'll fix later.

Comment by Michael Blume [ 29/Dec/14 2:49 PM ]

I don't know if this is supposed to be allowed, but this breaks

(merge {} [:foo 'bar])

which is used in the wild by compojure-api

Comment by Michael Blume [ 29/Dec/14 2:49 PM ]

https://github.com/metosin/compojure-api/blob/0.16.6/src/compojure/api/meta.clj#L198

Comment by Michael Blume [ 29/Dec/14 2:54 PM ]

Ghadi, contains? uses get under the covers, so it's still two gets, right? It seems like it'd be more performant to stick with the ::none trick.

Comment by Nicola Mometto [ 29/Dec/14 5:36 PM ]

This calls for if-let + find.

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

new patch addressing concerns so far

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

CLJ-1458-transient-merge3.patch removes silly inlining macro, uses singleton fns instead.

Comment by Michael Blume [ 29/Dec/14 11:14 PM ]

Nice =)

This should come with tests. If we want to preserve the ability to merge with a MapEntry, we should test it. This isn't so much a weakness of the patch as of the existing tests. I see merge and merge-with being used a few times in the test suite, but I see no test whose purpose is to test their behavior.

Comment by Michael Blume [ 29/Dec/14 11:17 PM ]

Extremely simple merge test, we need more than this, but this is a start

Comment by Alex Miller [ 22/Jun/15 10:11 AM ]

clj-1458-4.patch refreshed to apply to master, no changes.





[CLJ-1107] 'get' should throw exception on non-Associative argument Created: 13/Nov/12  Updated: 23/Feb/15

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

Type: Enhancement Priority: Critical
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Unresolved Votes: 15
Labels: checkargs

Attachments: Text File 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch     Text File 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch     Text File clj-1107-throw-on-unsupported-get-v4.patch    
Patch: Code and Test
Approval: Triaged

 Description   

The implementation of clojure.core/get returns nil if its argument is not an associative collection.

This behavior can obscure common programmer errors such as:

(def a (atom {:a 1 :b 2})

(:foo a)   ; forgot to deref a
;;=> nil

Calling get on something which is neither nil nor an Associative collection is almost certainly a bug, and should be indicated by an exception.

CLJ-932 was accepted as a similar enhancement to clojure.core/contains?

Patch: 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch

Approach: Throw IllegalArgumentException as final fall-through case in RT.getFrom instead of returning nil.



 Comments   
Comment by Andy Fingerhut [ 24/May/13 12:31 PM ]

Patch clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt dated May 24 2013 is identical to 0001-CLJ-1107-Throw-exception-for-get-called-on-unsupport.patch dated Nov 13 2012, except it applies cleanly to latest master. A recent commit for CLJ-1099 changed many IllegalArgumentException occurrences to Throwable in the tests, which is the only thing changed in this updated patch.

Comment by Andy Fingerhut [ 30/Jan/14 5:01 PM ]

Patch clj-1107-throw-on-get-for-unsupported-types-patch-v2.txt applied cleanly to latest Clojure master as of Jan 23 2014, but no longer does with commits made to Clojure between then and Jan 30 2014. I have not checked to see how difficult or easy it may be to update this patch.

Comment by Stuart Sierra [ 11/Feb/14 7:23 AM ]

New patch 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch created from master at 5cc167a.

Comment by Andy Fingerhut [ 26/Mar/14 11:55 AM ]

Patch clj-1107-throw-on-unsupported-get-v4.patch dated Mar 26 2014 is identical to Stuart Sierra's patch 0003-CLJ-1107-Throw-exception-for-get-on-unsupported-type.patch, and retains his authorship. The only difference is in one line of diff context required in order to make it apply cleanly to latest master.

Comment by Rich Hickey [ 10/Jun/14 10:54 AM ]

This would be a breaking change

Comment by Stuart Sierra [ 17/Jun/14 6:59 PM ]

Arguably so was CLJ-932 (contains?), which did "break" some things that were already broken.

This is a more invasive change than CLJ-932, but I believe it is more likely to expose hidden bugs than to break intentional behavior.

Comment by Andy Sheldon [ 07/Oct/14 5:40 AM ]

Is it more idiomatic to use "({:a 1}, :a)" and a safe replacement to boot? E.g. could you mass replace "(get " with "(" in a code base, in order to find bugs? I am still learning the language, and not young anymore, and couldn't reliably remember the argument order. So, I found it easier to avoid (get) with maps anyways. Without it I can put the map first or second.





[CLJ-1610] Unrolled small maps Created: 08/Dec/14  Updated: 31/Jul/15

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

Type: Enhancement Priority: Critical
Reporter: Alex Miller Assignee: Zach Tellman
Resolution: Unresolved Votes: 6
Labels: collections

Approval: Vetted

 Description   

Placeholder for unrolled small maps enhancement (companion for vectors at CLJ-1517).



 Comments   
Comment by Andy Fingerhut [ 09/Jul/15 10:59 PM ]

Is there an expectation that these would perform better that PersistentArrayMap?

Comment by Zach Tellman [ 09/Jul/15 11:53 PM ]

Yes, in some cases significantly so, for three reasons (in rough order of importance):

  • positional constructors, without any need for array instantiation/population
  • short-circuiting equality checks using hash comparisons
  • no iteration on any operation

There are a series of benchmarks at https://github.com/ztellman/cambrian-collections/blob/master/test/cambrian_collections/map_test.clj#L64-L148, which compare operations against maps with both keywords (which don't benefit from the hash comparisons) and symbols (which do). The 7-entry map cases cause the unrolled maps to overflow, so they only exist to test the overflow mechanism.

I've run the benchmark suite on my laptop, and the results are at https://gist.github.com/ztellman/961001e1a77e4f76ee1d. Some notable results:

The rest of the benchmarks are marginally faster due to unrolling, but most of the performance benefits are from the above behaviors. In a less synthetic benchmark, I found that Cheshire JSON decoding (which is 33% JSON lexing and 66% data structure building) was sped up roughly 30-40%.





[CLJ-1293] Portable "catch-all" mechanism Created: 05/Nov/13  Updated: 28/Dec/14

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

Type: Enhancement Priority: Critical
Reporter: Brandon Bloom Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: None

Attachments: Text File CLJ-1293-v001.patch    
Patch: Code and Test
Approval: Triaged

 Description   

Design page: http://dev.clojure.org/display/design/Platform+Errors

CLJS ticket/patch: http://dev.clojure.org/jira/browse/CLJS-661

This patch is more permissive than my patch for CLJS: The CLJS patch ensures :default catch blocks occur between non-default catch blocks and finally blocks, if present. This patch just makes (catch :default ...) a synonym for (catch Throwable ...). I wanted to keep the change to the compiler minimum.



 Comments   
Comment by Brandon Bloom [ 28/Dec/14 11:33 AM ]

Noticed this switched from "Minor" to "Critical", so I figured I should mention that I later realized that we might want :default to catch Exception instead of Throwable, so as to avoid catching Error subclasses. Javadocs say: "An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch." If that's what we actually want, I can provide an updated patch.

Comment by Alex Miller [ 28/Dec/14 2:19 PM ]

Seems like an open question, might be best just to list it as such in the description.

I don't really expect to reach consensus on the ticket or patch right now, just trying to update priorities and raise visibility for discussion with Rich once we get to 1.8.





[CLJ-1793] Clear 'this' before calls in tail position Created: 05/Aug/15  Updated: 24/Aug/15

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

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

1.8.0-alpha2 - 1.8.0-alpha4


Attachments: Text File 0001-Clear-this-before-calls-in-tail-position.patch     Text File clj-1793-2.patch    
Patch: Code and Test
Approval: Vetted

 Description   

(This ticket started life as CLJ-1250, was committed in 1.8.0-alpha2, pulled out after alpha4, and this is the new version that fixes the logic about whether in a tail call as well as addresses direct linking added in 1.8.0-alpha3.)

Problem: Original example was with reducers holding onto the head of a lazy seq:

(time (reduce + 0 (map identity (range 1e8))))    ;; works
(time (reduce + 0 (r/map identity (range 1e8))))  ;; oome from holding head of range

Example of regression from CLJ-1250 that now works correctly (doesn't clear this in nested loop) with CLJ-1793:

(let [done (atom false)
        f (future-call
            (fn inner []
              (while (not @done)
                (loop [found []]
                  (println (conj found 1))))))]
    (doseq [elem [:a :b :c :done]]
      (println "queue write " elem))
    (reset! done true)
    @f)

Approach: When invoking a method in a tail call, clear this prior to invoking.

The criteria for when a tail call is a safe point to clear 'this':

1) Must be in return position
2) Not in a try block (might need 'this' during catch/finally)
3) When not direct linked

Return position (#1) isn't simply (context == C.RETURN) because loop bodies are always parsed in C.RETURN context

A new dynvar METHOD_RETURN_CONTEXT tracks whether an InvokeExpr in tail position can directly leave the body of the compiled java method. It is set to RT.T in the outermost parsing of a method body and invalidated (set to null) when a loop body is being parsed where the context for the loop expression is not RETURN parsed. Added clear in StaticInvokeExpr as that is now a thing with direct linking again.

Removes calls to emitClearLocals(), which were a no-op.

Patch: clj-1793-2.patch

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 05/Aug/15 12:16 PM ]

The this ref is cleared prior to the println, but the next time through the while loop it needs the this ref to look up the closed over done field (via getfield).

Adding an additional check to the inTailCall() method to not include tail call in a loop addresses this case:

static boolean inTailCall(C context) {
-    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null);
+    return (context == C.RETURN) && (IN_TRY_BLOCK.deref() == null) && (LOOP_LOCALS.deref() == null);
}

But want to check some more things before concluding that's all that's needed.

Comment by Alex Miller [ 05/Aug/15 1:36 PM ]

This change undoes the desired behavior in the original CLJ-1250 (new tests don't pass). For now, we are reverting the CLJ-1250 patch in master.

Comment by Ghadi Shayban [ 05/Aug/15 3:12 PM ]

Loop exit edges are erroneously being identified as places to clear 'this'. Only exits in the function itself or the outermost loop are safe places to clear.

Comment by Ghadi Shayban [ 05/Aug/15 8:43 PM ]

Patch addresses this bug and the regression in CLJ-1250.

See the commit message for an extensive-ish comment.

Comment by Alex Miller [ 18/Aug/15 12:33 PM ]

New patch is same as old, just adds jira id to beginning of commit message.

Comment by Rich Hickey [ 24/Aug/15 10:00 AM ]

Not doing this for 1.8, more thought needs to go into whether this is the right solution to the problem. And, what is the problem? This title of this patch is just something to do.

Comment by Alex Miller [ 24/Aug/15 10:21 AM ]

changing to vetted so this is at a valid place in the jira workflow

Comment by Ghadi Shayban [ 24/Aug/15 10:45 AM ]

Rich the original context is in CLJ-1250 which was a defect/problem. It was merged and revert because of a problem in the impl. This ticket is the continuation of the previous one, but unfortunately the title lost the context and became approach-oriented and not problem-oriented. Blame Alex. (I kid, it's an artifact of the mutable approach to issue management.)





[CLJ-705] Make sorted maps and sets implement j.u.SortedMap and SortedSet interfaces Created: 05/Jan/11  Updated: 02/Jun/12

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

Type: Enhancement Priority: Major
Reporter: Rich Hickey Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Comments   
Comment by Andy Fingerhut [ 02/Jun/12 2:29 PM ]

This might be a duplicate of CLJ-248. See that one before working on this one, at least.





[CLJ-1108] Allow to specify an Executor instance to be used with future-call Created: 18/Nov/12  Updated: 27/Dec/12

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

Type: Enhancement Priority: Major
Reporter: Max Penet Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File bac37b91230d8e4ab3a1e6042a6e8c4b7e9cbf53.patch     Text File clj-1108-enhance-future-call-patch-v2.txt    
Patch: Code

 Description   

This adds an arity to future-call that expects a java.util.concurrent/ExecutorService instance to be used instead of clojure.lang.Agent/soloExecutor.



 Comments   
Comment by Andy Fingerhut [ 26/Dec/12 4:50 PM ]

Rich Hickey committed a change on Dec 21, 2012 to the future-call function that made the patch bac37b91230d8e4ab3a1e6042a6e8c4b7e9cbf53.patch dated Nov 18 2012 no longer apply cleanly.

clj-1108-enhance-future-call-patch-v2.txt dated Dec 26 2012 is identical to that earlier patch, except it has been updated to apply cleanly to the latest master.

It would be best if Max Penet, author of the earlier patch, could verify I've merged his patch to the latest Clojure master correctly.

Comment by Max Penet [ 27/Dec/12 2:25 AM ]

It's verified, it applies correctly to the latest master 00978c76edfe4796bd6ebff3a82182e235211ed0 .

Thanks Andy.





[CLJ-859] Built in dynamic vars don't have :dynamic metadata Created: 19/Oct/11  Updated: 24/Feb/12

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

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


 Description   

I'm sure 'built in' is probably not the right term here, but I'm not sure what these are called.

I ran into this issue earlier today while fixing a bug in clojail. Built in vars, particularly ones listed here without a source link: http://clojure.github.com/clojure/clojure.core-api.html, do not have :dynamic metadata despite being dynamic. This includes *in*, *out*, and *err* among others. Here are some examples:

user=> (meta #'*err*)
{:ns #<Namespace clojure.core>, :name *err*, :added "1.0", :doc "A java.io.Writer object representing standard error for print operations.\n\n  Defaults to System/err, wrapped in a PrintWriter"}
user=> (meta #'*in*)
{:ns #<Namespace clojure.core>, :name *in*, :added "1.0", :doc "A java.io.Reader object representing standard input for read operations.\n\n  Defaults to System/in, wrapped in a LineNumberingPushbackReader"}
user=> (meta #'*out*)
{:ns #<Namespace clojure.core>, :name *out*, :added "1.0", :doc "A java.io.Writer object representing standard output for print operations.\n\n  Defaults to System/out, wrapped in an OutputStreamWriter", :tag java.io.Writer}
user=> (meta #'*ns*)
{:ns #<Namespace clojure.core>, :name *ns*, :added "1.0", :doc "A clojure.lang.Namespace object representing the current namespace.", :tag clojure.lang.Namespace}


 Comments   
Comment by Ben Smith-Mannschott [ 19/Oct/11 12:03 PM ]

This recent discussion on the users list seems relevant: Should intern obey :dynamic?.

It seems to boil down to this the information that a Var is dynamic (or not) is duplicated. Once as metadata with the key :dynamic, and once as a boolean field on the Var class which implements Clojure's variables. This boolean can be obtained by calling the method isDynamic() on the Var.

The confusion arises because apparently :dynamic and .isDynamic can get out of sync with each other. .isDynamic is the source of truth in this case.

Comment by Ben Smith-Mannschott [ 19/Oct/11 12:18 PM ]

Compiler$Parser.parse(...) finds the :dynamic entry left in the metadata of the symbol by LispReader and passes this on when creating a new DefExpr, which in turn, generates the code that will call setDynamic(...) on the var when it is created at runtime.

As far as I can tell, the :dynamic entry is irrelevant once that has occurred. It seems to be implemented only as a way to communicate (by way of the reader) with the compiler. Once the compiler's gotten the message, it isn't needed anymore. Keeping it around seems to just cause confusion.

Dynamic vars created by the Java layer of Clojure core don't use the :dynamic mechanism, they just setDynamic() directly. That's why they don't have :dynamic in their meta-data map.

  • Perhaps the compiler should elide :dynamic from the metadata map available at runtime, since it has served its purpose.
  • Perhaps Clojure should supply the function dynamic?.
    (defn dynamic? [^clojure.lang.Var v] (.isDynamic v))

Or, perhaps one might consider, for 1.4, replacing :dynamic altogether and just enforcing the established naming convention: *earmuffs* are dynamic, everything-else isn't. (The compile warns about violations of this convention in 1.3.)

Comment by Andy Fingerhut [ 24/Feb/12 11:39 AM ]

I recently noticed several lines like this one in core.clj. Depending upon how many symbols are like this, perhaps this method could be used to add :dynamic metadata to symbols in core, along with a unit test to verify that all symbols in core have :dynamic if and only if .isDynamic returns true?

Comment by Andy Fingerhut [ 24/Feb/12 12:41 PM ]

Ugh. In my previous comment, by "several lines like this one" I meant to paste the following as an example:

(alter-meta! #'agent assoc :added "1.0")





[CLJ-1016] Global scope overrides lexical scope for classes (Clojure assumes no classes in default package / Clojure cannot handle yFiles JARs in classpath) Created: 21/Jun/12  Updated: 24/May/13

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

Type: Defect Priority: Major
Reporter: Edward Z. Yang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File collision-workaround.patch    

 Description   

The most visible symptom of this bug is having a class named 'w' (default package) in your classpath (such classes are produced by Java obfuscation tools such as yFiles) and then attempting to load Clojure's core class. For example:

java -cp hotspotapi.jar:clojure-1.4.0-slim.jar clojure.main

(where hotspotapi.jar is a stereotypical example of an obfuscated JAR) results in:

Exception in thread "main" java.lang.ExceptionInInitializerError
at clojure.main.<clinit>(main.java:20)
Caused by: java.lang.NoSuchFieldException: close, compiling:(clojure/core.clj:6139)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6462)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$TryExpr$Parser.parse(Compiler.java:2178)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5919)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6443)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6443)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5618)
at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5054)
at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3674)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6453)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6443)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.access$100(Compiler.java:37)
at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:518)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
at clojure.lang.Compiler.analyze(Compiler.java:6262)
at clojure.lang.Compiler.analyze(Compiler.java:6223)
at clojure.lang.Compiler.eval(Compiler.java:6515)
at clojure.lang.Compiler.load(Compiler.java:6952)
at clojure.lang.RT.loadResourceScript(RT.java:359)
at clojure.lang.RT.loadResourceScript(RT.java:350)
at clojure.lang.RT.load(RT.java:429)
at clojure.lang.RT.load(RT.java:400)
at clojure.lang.RT.doInit(RT.java:436)
at clojure.lang.RT.<clinit>(RT.java:318)
... 1 more
Caused by: java.lang.NoSuchFieldException: close
at java.lang.Class.getField(Class.java:1537)
at clojure.lang.Compiler$StaticFieldExpr.<init>(Compiler.java:1180)
at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:923)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:6455)
... 37 more
Could not find the main class: clojure.main. Program will exit.

To understand what is going on, consider this simple test:

import java.io.StringReader;

import clojure.lang.Compiler;
import clojure.lang.RT;

public class Test {
public static void main(String[] args) { RT.var("clojure.core", "require"); String s = "(let [mumble (new java.io.StringReader \"\")] (. mumble close))"; Compiler.load(new StringReader(s)); }
}

It should be clear that 'mumble' in the dot operator is referencing the locally defined mumble. However, if we define a class named 'mumble' in the default package, Clojure picks that one up instead.

To forestall any objections: yes, we know that placing classes in the default package is extremely poor form. Point of the matter is, the Java ecosystem is extremely diverse and there are a lot of JARs people may not have control over. While one might argue, "Don't put classes in the default namespace", point of the matter is, Clojure is wrong here, and these situations arise in practice, through no fault of the implementer.



 Comments   
Comment by Edward Z. Yang [ 21/Jun/12 11:01 AM ]

Here is a workaround patch which makes this error less likely to occur.

Comment by Andy Fingerhut [ 27/Aug/12 7:37 PM ]

Edward, it is Rich Hickey's policy only to consider for inclusion in Clojure patches written by people who have signed a Contributor Agreement: http://clojure.org/contributing

Were you interested in becoming a contributor?

Comment by Edward Z. Yang [ 27/Aug/12 9:24 PM ]

Sure, although the patch attached is emphatically not the one you want to actually applying, since it only band-aids the problem.

Comment by Andy Fingerhut [ 24/May/13 1:21 PM ]

I am not sure, but this ticket may be related to CLJ-1171. At least, there the issue was a global name not being shadowed by a local name bound with let. That seems similar to this issue.





[CLJ-112] GC Issue 108: All Clojure interfaces should specify CharSequence instead of String when possible Created: 17/Jun/09  Updated: 26/Aug/13

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

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


 Description   
Reported by redchin, Apr 20, 2009

rhickey: unlink: then just use a map {:escaped true :val "foo"}

unlink: What I meant is, everything in between would want to see something 
String-y, not caring whether it's a String or MyString.

hiredman: unlink: if you use something that implements CharSequence and 
IMeta (I think it's IMeta) you get something that is basically a String, 
but with metadata

rhickey: what hiredman said

hiredman: ideally most things would not specify String but CharSequence in 
their interface

hiredman: but somehow I doubt that is case

unlink: ok.

unlink: Good to know.

rhickey: hiredman: unfortunately that's not true of some of Clojure - could 
you enter an issue for it please - use CharSequence when possible?


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

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

Comment by Assembla Importer [ 24/Aug/10 3:45 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)





[CLJ-113] GC Issue 109: RT.load's "don't load if already loaded" mechanism breaks ":reload-all" Created: 17/Jun/09  Updated: 26/Aug/13

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

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Stephen C. Gilardi
Resolution: Unresolved Votes: 0
Labels: None


 Description   
Reported by scgilardi, Apr 24, 2009

What (small set of) steps will reproduce the problem?

"require" and "use" support a ":reload-all" flag that is intended to  
cause the specified libs to be reloaded along with all libs on which  
they directly or indirectly depend. This is implemented by temporarily  
binding a "loaded-libs" var to the empty set and then loading the  
specified libs.

AOT compilation added another "already loaded" mechanism to  
clojure.lang.RT.load() which is currently not sensitive to a "reload-
all" being in progress and breaks its operation in the following case:

        A, B, and C are libs
        A depends on B. (via :require in its ns form)
        B depends on C. (via :require in its ns form)
        B has been compiled (B.class is on classpath)

        At the repl I "require" A which loads A, B, and C (either from
class files or clj files)
        I modify C.clj
        At the repl I "require" A with the :reload-all flag, intending to  
pick up the changes to C
        C is not reloaded because RT.load() skips loading B: B.class
exists, is already loaded, and B.clj hasn't changed since it was compiled.


What is the expected output? What do you see instead?

I expect :reload-all to be effective. It isn't.

What version are you using?

svn 1354, 1.0.0RC1

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

http://groups.google.com/group/clojure/browse_frm/thread/9bbc290321fd895f/e6a967250021462a#e6a967250021462a

Please provide any additional information below.

I'll upload a patch soon that creates a "*reload-all*" var with a  
root binding of nil and code to bind it to true when the current  
thread has a :reload-all call pending. When *reload-all* is true,  
RT.load() will (re)load all libs from their ".clj" files even if  
they're already loaded.

The fix for this may need to be coordinated with a fix for issue #3.


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

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

Comment by Assembla Importer [ 24/Aug/10 3:45 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 [ 24/Aug/10 3:45 AM ]

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

Comment by Kevin Downey [ 08/Aug/11 7:40 PM ]

seems like the code that is emitted in the static init for namespace classes could be emitted into a init_ns() static method and the static init could call init_ns(). then RT.load could call init_ns() to get the behavior of reloading an AOT compiled namespace.

Comment by Kevin Downey [ 09/Aug/11 8:31 PM ]

looking at the compiler it looks like most of what I mentioned above is already implemented, just need RT to reflectively call load() on the namespace class in the right place





[CLJ-396] Better support for multiple inheritance in hierarchies and multimethods Created: 07/Jul/10  Updated: 26/Aug/13

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

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


 Description   

While the hierarchies produced with 'derive' allow multiple parents per child, there is no way to indicate precedence between those parents, other than by laboriously specifying 'prefer-method' for every type X every multimethod. When 2 multimethods are both applicable to the supplied arguments, Clojure produces a nonspecific IllegalArgumentException containing only an error string. All this means that while Clojure does have an "inheritance" mechanism in the form of the ad hoc hierarchies, it is currently not really possible to implement multiple inheritance using the ad hoc hierarchy mechanism. 'Prefer-method' will not scale up to use in large applications with complex type hierarchies and heavy use of multimethods.

Some potential ways to solve this are:

  • allowing 'defmulti' to take a 'tie-breaker' function (tie-breaker [arglist speclist1 speclist2] ...) which is called instead of throwing an IllegalArgumentException, and must return the 'winning speclist'.
  • instead of throwing IllegalArgumentException, throw a TiedMultiMethodsException – the exception instance should contain the offending speclists, the function, and the arguments that were supplied.
  • allowing specification of precedence when using 'derive' (if only via a "last in = highest precedence" rule).

Paul



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

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





[CLJ-115] GC Issue 111: Enable naming an array parameter for areduce Created: 17/Jun/09  Updated: 26/Aug/13

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

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


 Description   
Reported by bo...@boriska.com, Apr 28, 2009

Currently there is no way to access anonymous array parameter of areduce.

Consider:

(areduce (.. System getProperties values toArray) 
     i r 0 (some_expression))
some_expression has no way to access the array.

Per Rich:
--------------------
Yes, areduce would be nicer if it looked like a binding set:

(areduce [aname anarray, ret init] expr)
(areduce [aname anarray, ret init, start-idx  start-n] expr)
(areduce [aname anarray, ret init, start-idx  start-n, end-idx end-n]
expr) 
--------------------

This was discussed here:
http://groups.google.com/group/clojure/tree/browse_frm/thread/40597a8ac322bc37/8cf6b17328ea7e8b?rnum=1&_done=%2Fgroup%2Fclojure%2Fbrowse_frm%2Fthread%2F40597a8ac322bc37%2F8cf6b17328ea7e8b%3Ftvc%3D1%26pli%3D1%26#doc_9ea7e3c5d500ed3c


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

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

Comment by Assembla Importer [ 24/Aug/10 5:45 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)





[CLJ-319] TransactionalHashMap bug Created: 26/Apr/10  Updated: 26/Aug/13

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

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


 Description   

TransactionalHashMap computation of the bin is buggy. The implementation doesn't unset the sign bit before using it in accessing the bin array which in some cases cause an ArrayOutOfBoundException to be thrown.

As Rich Hickey has pointed out, this is an unsupported experimental Class and won't be fixed unless I provided a patch, so attached is the patch file.



 Comments   
Comment by Assembla Importer [ 01/Oct/10 4:06 PM ]

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

Comment by Assembla Importer [ 01/Oct/10 4:06 PM ]

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

Comment by Assembla Importer [ 01/Oct/10 4:06 PM ]

stu said: Please add a test case.





[CLJ-21] GC Issue 17: arity checking during compilation Created: 17/Jun/09  Updated: 26/Aug/13

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

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


 Description   
Reported by richhickey, Dec 17, 2008
Use available metadata to check calls when possible


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

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





[CLJ-42] GC Issue 38: When using AOT compilation, "load"ed files are not reloaded on (require :reload 'name.space) Created: 17/Jun/09  Updated: 26/Aug/13

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

Type: Defect Priority: Major
Reporter: Anonymous Assignee: Stephen C. Gilardi
Resolution: Unresolved Votes: 0
Labels: None


 Description   
Reported by m...@kotka.de, Jan 07, 2009
What (small set of) steps will reproduce the problem?

1. Create a file src/foo.clj

cat >src/foo.clj <<EOF
(ns foo (:load "bar"))
EOF

2. Create a file src/bar.clj

cat >src/bar.clj <<EOF
(clojure.core/in-ns 'foo)
(def x 8)
EOF

3. Start Clojure Repl: java -cp src:classes clojure.main -r

4. Compile the namespace.

user=> (compile 'foo)
foo

5. Require the namespace
user=> (require :reload-all :verbose 'foo)
(clojure.core/load "/foo")
(clojure.core/load "/bar")

What is the expected output? What do you see instead?

6. Re-Require the namespace

user=> (require :reload-all :verbose 'foo)
(clojure.core/load "/foo")

Only the "master" file is loaded, but not the bar file.
Expected would have been to also load the bar file.
Changes to bar.clj are not reflected, and depending
on the setting (eg. using multimethods in foo from
a different namespace) code may be corrupted.

What version are you using?

SVN rev. 1195


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

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

Comment by Assembla Importer [ 24/Aug/10 6:44 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 [ 24/Aug/10 6:44 AM ]

richhickey said: Updating tickets (#42, #71)

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

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





[CLJ-213] Invariants and the STM Created: 01/Dec/09  Updated: 26/Aug/13

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

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


 Description   

(ticket requested here http://groups.google.com/group/clojure/browse_thread/thread/119311e89fa46806/4903ce25ff6deaa6#4903ce25ff6deaa6)

The general idea is to declare invariants inside a transaction and, when at commit time an invariant doesn't hold anymore, the transaction retries.
So it can both act as a kind of soft ensure or to specify actions that "partially commute".
Thus it would enable coarser refs.

See the attached file for quick prototype.

User code would looks like:

(invariant (@world :key))
(commute world update-in [:key] val-transform-fn)

This means the commute will occur only if (@world :key) returns the same value in-transaction and at commit point.



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

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





[CLJ-1136] Type hinting for array classes does not work in binding forms Created: 20/Dec/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop, typehints
Environment:

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



 Description   

Type hints don't work as expected in binding forms.

The following form results in a reflection warning:

(let [^{:tag (Class/forName "[Ljava.lang.Object;")} a (make-array Object 2)]
(aget a 0))

However, hinting does appear to work correctly on vars:

(def ^{:tag (Class/forName "[Ljava.lang.Object;")} a (make-array Object 2))
(aget a 0) ;; no reflection warning



 Comments   
Comment by Ghadi Shayban [ 20/Dec/12 10:51 PM ]

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

Comment by Ghadi Shayban [ 21/Dec/12 11:09 AM ]

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





[CLJ-1001] Proxy cannot call proper super-class method Created: 23/May/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Guanpeng Xu Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop
Environment:

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


Attachments: File proxy-bug.clj    

 Description   

Attached is a program that reproduces this issue. We have a proxy, `p', which sub-classes java.io.InputStream. There are three methods named `read' in java.io.InputStream: abstract int read(); int read(byte[] b); and int read(byte[] b, int off, int len); see http://docs.oracle.com/javase/6/docs/api/java/io/InputStream.html. In the definition of proxy `p', we implement the abstract variant of method `read', making `p' a concrete instance of java.io.InputStream.

The first invocation, (. p read), returns -1, which is expected.

The second invocation, (. p (read b 0 n)), should call int read(byte[] b, int off, int len); in java.io.InputStream. But these are actual behavior:

$ clojure1.2 ~/tmp/proxy-bug.clj
Exception in thread "main" java.lang.IllegalArgumentException: Wrong number of args (4) passed to: user$eval1$fn (proxy-bug.clj:0)
at clojure.lang.Compiler.eval(Compiler.java:5441)
at clojure.lang.Compiler.load(Compiler.java:5858)
at clojure.lang.Compiler.loadFile(Compiler.java:5821)
at clojure.main$load_script.invoke(main.clj:221)
at clojure.main$script_opt.invoke(main.clj:273)
at clojure.main$main.doInvoke(main.clj:354)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.lang.Var.invoke(Var.java:365)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.Var.applyTo(Var.java:482)
at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: 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:5425)
... 10 more

$ clojure1.2 ~/tmp/proxy-bug.clj
Exception in thread "main" java.lang.IllegalArgumentException: Wrong number of args (4) passed to: user$eval1$fn (proxy-bug.clj:0)
at clojure.lang.Compiler.eval(Compiler.java:5441)
at clojure.lang.Compiler.load(Compiler.java:5858)
at clojure.lang.Compiler.loadFile(Compiler.java:5821)
at clojure.main$load_script.invoke(main.clj:221)
at clojure.main$script_opt.invoke(main.clj:273)
at clojure.main$main.doInvoke(main.clj:354)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.lang.Var.invoke(Var.java:365)
at clojure.lang.AFn.applyToHelper(AFn.java:161)
at clojure.lang.Var.applyTo(Var.java:482)
at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: 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:5425)
... 10 more



 Comments   
Comment by Guanpeng Xu [ 23/May/12 10:24 PM ]

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.

Comment by Russell Mull [ 01/Sep/13 3:12 AM ]

Verified with Clojure 1.5.1:

Stack Trace
clojure.lang.ArityException: Wrong number of args (4) passed to: user$eval147$fn
                                      AFn.java:437 clojure.lang.AFn.throwArity
                                       AFn.java:51 clojure.lang.AFn.invoke
                                  (Unknown Source) user.proxy/java.io.InputStream[fn]
                                  NO_SOURCE_FILE:9 user/eval147
                                Compiler.java:6619 clojure.lang.Compiler.eval
                                Compiler.java:6582 clojure.lang.Compiler.eval
                                     core.clj:2852 clojure.core/eval
                                      main.clj:259 clojure.main/repl[fn]
                                      main.clj:259 clojure.main/repl[fn]
                                      main.clj:277 clojure.main/repl[fn]
                                      main.clj:277 clojure.main/repl
                                  RestFn.java:1096 clojure.lang.RestFn.invoke
                         interruptible_eval.clj:56 clojure.tools.nrepl.middleware.interruptible-eval/evaluate[fn]
                                      AFn.java:159 clojure.lang.AFn.applyToHelper
                                      AFn.java:151 clojure.lang.AFn.applyTo
                                      core.clj:617 clojure.core/apply
                                     core.clj:1788 clojure.core/with-bindings*
                                   RestFn.java:425 clojure.lang.RestFn.invoke
                         interruptible_eval.clj:41 clojure.tools.nrepl.middleware.interruptible-eval/evaluate
                        interruptible_eval.clj:171 clojure.tools.nrepl.middleware.interruptible-eval/interruptible-eval[fn]
                                     core.clj:2330 clojure.core/comp[fn]
                        interruptible_eval.clj:138 clojure.tools.nrepl.middleware.interruptible-eval/run-next[fn]
                                       AFn.java:24 clojure.lang.AFn.run
                      ThreadPoolExecutor.java:1110 java.util.concurrent.ThreadPoolExecutor.runWorker
                       ThreadPoolExecutor.java:603 java.util.concurrent.ThreadPoolExecutor$Worker.run
                                   Thread.java:722 java.lang.Thread.run




[CLJ-1022] gen-class destroys method annotations Created: 03/Jul/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Maris Orbidans Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop


 Description   

When extending a class gen-class doesn't preserve method annotations.

If class com.bar.Foo has annotated methods then in MyClass all annotations are gone.

(gen-class
:name com.my.MyClass
:extends com.bar.Foo
:implements [com.google.common.base.Supplier]
:prefix demo-
:post-init post-init)

(defn demo-post-init [this]
(info "initialized")
(swank.swank/start-server :port 68478))

(defn demo-get [_]
(get-msg))

Class<?> aClass = Class.forName("com.my.MyClass");
Method[] methods = aClass.getMethods();

for (Method m : methods) {
Annotation[] annotations = m.getAnnotations();
System.out.println(m.getName()+" "+annotations.length);
for (Annotation a : annotations) { System.out.println(a.annotationType().getClass().getName()); }
}






[CLJ-994] repeat reducer Created: 11/May/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Jason Jackson Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reducers

Attachments: Text File 0001-repeat-for-clojure.core.reducers.patch    
Patch: Code and Test

 Description   

i'm working on clojure.core/repeat reducer.



 Comments   
Comment by Andy Fingerhut [ 17/May/12 6:18 PM ]

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.

Comment by Jason Jackson [ 17/May/12 6:41 PM ]

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.

Comment by Jason Jackson [ 19/May/12 2:55 PM ]

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.

Comment by Andy Fingerhut [ 20/May/12 3:00 AM ]

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?

Comment by Jason Jackson [ 20/May/12 11:55 AM ]

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

Comment by Andy Fingerhut [ 08/Jun/12 7:11 PM ]

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.

Comment by Jason Jackson [ 14/Aug/12 1:17 AM ]

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

Comment by Jason Jackson [ 14/Sep/12 2:37 PM ]

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





[CLJ-993] `range` reducer Created: 10/May/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: reducers

Attachments: Text File 0001-CLJ-993-implement-range-and-iterate-as-reducers.patch     Text File 0001-CLJ-993-implement-range-and-iterate-as-reducers.patch     Text File 0001-CLJ-993-implement-range-as-a-reducer.patch     Text File 0002-Make-iterate-and-range-Seqable.patch     Text File 0003-Implement-fold-for-Range-objects.patch     Text File just-iseq.patch     Text File range-reducer.patch    
Patch: Code and Test

 Description   

Rich mentioned in IRC today he'd welcome a reducer implementation of clojure.core/range. Now that I've figured out how to do iterate, I figure I'll knock out range as well by the end of the night. Just opening the issue early to announce my intentions to anyone else interested in doing it.



 Comments   
Comment by Alan Malloy [ 10/May/12 10:45 PM ]

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.

Comment by Jason Jackson [ 11/May/12 11:20 AM ]

Range should be foldable

Comment by Alan Malloy [ 11/May/12 12:53 PM ]

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

Comment by Alan Malloy [ 11/May/12 2:42 PM ]

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.

Comment by Rich Hickey [ 11/May/12 3:21 PM ]

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.

Comment by Alan Malloy [ 11/May/12 3:30 PM ]

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?

Comment by Rich Hickey [ 11/May/12 3:52 PM ]

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!

Comment by Stuart Sierra [ 11/May/12 5:01 PM ]

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.

Comment by Alan Malloy [ 11/May/12 9:00 PM ]

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.

Comment by Alan Malloy [ 11/May/12 11:52 PM ]

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

Comment by Alan Malloy [ 11/May/12 11:58 PM ]

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.

Comment by Rich Hickey [ 12/May/12 10:52 AM ]

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.

Comment by Alan Malloy [ 12/May/12 5:53 PM ]

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.

Comment by Alan Malloy [ 12/May/12 7:49 PM ]

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.

Comment by Rich Hickey [ 13/May/12 8:50 AM ]

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.

Comment by Jason Jackson [ 13/May/12 9:24 AM ]

Should r/range return something Seqable and Counted?

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

Comment by Alan Malloy [ 13/May/12 1:59 PM ]

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.

Comment by Alan Malloy [ 23/May/12 7:54 PM ]

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.

Comment by Rich Hickey [ 14/Aug/12 2:07 PM ]

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.

Comment by Alan Malloy [ 18/Aug/12 6:19 PM ]

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.

Comment by Alan Malloy [ 18/Aug/12 7:18 PM ]

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.





[CLJ-986] Adds an exit function to exit clojure process Created: 06/May/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

There is no standard function to exit the clojure process.
In java implementation,we use (System/exit 0),but in other implementations(CLR), i have to use another function.

Why not add a standard function in clojure.core?
For example:

(defn exit
([] (exit 0)
([status] (System/exit status)))

I think it's useful for us.






[CLJ-969] Symbol/keyword implements IFn for lookup but a non-collection argument produces non-intuitive results Created: 09/Apr/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Sean Corfield Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   

('+ 1 2) ;; return 2 because it is treated as (get 1 '+ 2)

Whilst this is "consistent" once you know the lookup behavior, it's confusing for Clojure newbies and it seems to be a non-useful behavior.

Proposal: modify Keyword.invoke() and Symbol.invoke() to restrict first Object argument to instanceof ILookup, Map or IPersistentSet (or null) so that the "not found" behavior doesn't produce non-intuitive behavior.






[CLJ-968] ns emitting gen-class before imports results in imported annotations being discarded. Created: 09/Apr/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Charles Duffy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop


 Description   

The following discards the imported annotations:

(ns com.example.BaseXModuleTest
  (:import (org.basex.query QueryModule QueryModule$Deterministic))
  (:gen-class
     :extends org.basex.query.QueryModule
     :methods [
       [^{QueryModule$Deterministic {}}
        addOne [int] int]]))

However, when moving the gen-class call out of the ns declaration, the annotation is correctly applied:

(ns com.example.BaseXModuleTest
  (:import (org.basex.query QueryModule QueryModule$Deterministic)))

(gen-class
  :extends org.basex.query.QueryModule
  :name com.example.BaseXModuleTest
  :methods [
    [^{QueryModule$Deterministic {}}
     addOne [int] int]])

It appears that imported names are not yet in-scope when gen-class is run from a ns declaration.






[CLJ-919] cannot create anonymous primitive functions Created: 27/Jan/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Ben Mabey Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: typehints


 Description   

Primitive functions only work (e.g. return primitive types) when defined with `defn`. An equivalent function created with `fn` does not behave the same way as when created with `defn`. For example:

(definterface IPrimitiveTester
(getType [^int x])
(getType [^long x])
(getType [^float x])
(getType [^double x])
(getType [^Object x]))

(deftype PrimitiveTester []
IPrimitiveTester
(getType [this ^int x] :int)
(getType [this ^long x] :long)
(getType [this ^float x] :float)
(getType [this ^double x] :double)
(getType [this ^Object x] :object))

(defmacro pt [x]
`(.getType (PrimitiveTester.) ~x))

(defn with-defn ^double [^double x]
(+ x 0.5))

(pt (with-defn 1.0)) ; => :double

(let [a (fn ^double [^double x] (+ x 0.5))]
(pt (a 0.1))) ; => :object

Please see the discussion on the mailing list for more details and thoughts on what is happening:
http://groups.google.com/group/clojure/browse_thread/thread/d83c8643a7c7d595?hl=en






[CLJ-911] 'proxy' prevents overriding Object.finalize (and doesn't document it) Created: 16/Jan/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Norman Gray Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop
Environment:

OS X, Java 1.6.0?



 Description   

It appears to be impossible to override Object.finalize() using proxy. If the method is defined using proxy, then it cannot be called straightforwardly (see below), and it is not called as a finalizer during normal program execution (not demonstrated below).

See extensive discussion at: https://groups.google.com/group/clojure/browse_thread/thread/a1e2fca45af6c1af

user=> (def m (proxy [java.util.HashMap] []
(finalize []
;(proxy-super finalize)
(prn "finalizing..."))
(hashCode []
99)))
#'user/m
user=> (.hashCode m)
99
user=> (.finalize m)
IllegalArgumentException No matching field found: finalize for class user.proxy$java.util.HashMap$0 clojure.lang.Reflector.getInstanceField (Reflector.java:289)

There is at least one of two bugs here (thanks to Cedric Greevey for summarising this way):

  • If the inability to override finalize() is unintentional, that's a bug.
  • If it's intentional for some reason, then (a) that's not documented, and (b) the failure is silent, in the sense that an explicit call produces an apparently completely unrelated error (above), and the failure to call the method during object finalization is completely silent.





[CLJ-903] extend-protocol does not allow classnames as a String Created: 30/Dec/11  Updated: 03/Sep/13

Status: Open
Project: Clojure
Component/s: None
Affects Version/s: Release 1.2, Release 1.3, Release 1.4
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Meikel Brandmeyer Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: interop


 Description   

In various places Clojure accepts classnames as String, eg. in gen-class or type hints. However it does not in extend-protocol. This does not allow simple specification of array types.

See also here: http://groups.google.com/group/clojure/browse_thread/thread/722a0c09d02bb0ac






[CLJ-434] Additional copy methods for URLs in clojure.java.io Created: 10/Sep/10  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: io


 Description   

The copy method in clojure.java.io doesn't handle java.net.URL as input.
The necessary methods can be found in the mailing list post:
[[url:http://groups.google.com/group/clojure/browse_thread/thread/24a105b12466a8e8]]



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

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





[CLJ-400] A faster flatten Created: 13/Jul/10  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance


 Description   

As discussed in this thread, I am submitting a more performant version of flatten for review. It has the same semantics as the current core/flatten. I have also updated the doc string to say that "(flatten nil) returns the empty list", because that's what the current version of core/flatten does as well.

I haven't mailed in a CA yet, but I will tomorrow morning.

Edit: Of course I'd mess my first ticket up . I am not immediately seeing an option to edit this to add the "patch" tag or add the "ready to test" action. Sorry folks



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

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





[CLJ-405] better error messages for bad defrecord calls Created: 20/Jul/10  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: defrecord, errormsgs


 Description   

defrecord could tell you if, e.g., you didn't specify an interface before leaping into method bodies. See http://groups.google.com/group/clojure/browse_thread/thread/f52f90954edd8b09



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

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

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

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.





[CLJ-277] Making clojure.xml/emit a little friendler to xml consumers Created: 03/Mar/10  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: xml


 Description   

Currently, clojure.xml/emit breaks the eBay api, because emit adds whitespace before and after :contents. This trivial patch fixes it for me:
http://github.com/tjg/clojure/commit/bbff079d26e627c655b847319a58d76b8b3cec7c

(Dunno whether there's a good reason emit works that way, or if I'm missing something obvious.)

I realize that emit's behavior conforms to the XML spec and it's probably eBay at fault here. But I can nevertheless see this whitespace causing problems.



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

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

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

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

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

stu said: Duplicated association with ticket #410 was added





[CLJ-252] Support typed non-primitive fields in deftype Created: 29/Jan/10  Updated: 03/Sep/13

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

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


 Description   

Right now hints are accepted but not used as field type.



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

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





[CLJ-233] better error reporting of nonexistent var Created: 31/Dec/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   

simple improvement to error message when referencing a var that doesn't exist.



 Comments   
Comment by Assembla Importer [ 29/Sep/10 5:29 AM ]

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

Comment by Assembla Importer [ 29/Sep/10 5:29 AM ]

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





[CLJ-190] enhance with-open to be extensible with a new close multimethod Created: 13/Sep/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: io


 Description   

Discussion: http://groups.google.com/group/clojure/browse_thread/thread/8e4e56f6fc65cc8e/618a893a5b2a5410

Currently, with-open calls .close when it's finished. I'd like it to have a (defmulti close type) so it's behavior is extensible. A standard method could be defined for java.io.Closeable and a :default method with no type hint. I've come across a few cases where some external library defines what is essentially a close method but names it shutdown or disable, etc., and adding my own "defmethod close" would be much easier than rewriting with-open. This would also allow people to eliminate reflection for classes like sql Connection that were created before Closeable.



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

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

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

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

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

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.

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

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

Comment by Tassilo Horn [ 23/Dec/11 6:50 AM ]

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





[CLJ-163] Enhance = and == docs Created: 30/Jul/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Laurent Petit
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

Enhance = and == docs as far as numbers handling is concerned (make them self referenced, make clear what == offers beyond = -except that it will only work for numbers)



 Comments   
Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/163
Attachments:
fixbug163.diff - https://www.assembla.com/spaces/clojure/documents/bH0XMCFjur3PLMeJe5aVNr/download/bH0XMCFjur3PLMeJe5aVNr

Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

laurentpetit said: [file:bH0XMCFjur3PLMeJe5aVNr]

Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

richhickey said: I don't want to recommend, in = doc, that people should prefer == for any case. People should always prefer =. If there is a perf, difference we can make that go away. Then the only difference with == is that it will fail on non-numbers, and that should be the only reason to choose it.

Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

richhickey said: Updating tickets (#94, #96, #104, #119, #163)

Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

laurentpetit said: Richn, by "will fail on non-numbers", do you mean "should throw an exception" (and thus the patch must change the code), or just as it works today :

(== :a :a)
false

?

Comment by Assembla Importer [ 24/Aug/10 1:04 PM ]

richhickey said: I've fixed the code so == on non-numbers throws





[CLJ-150] Doc for array-map should mention its characteristics/caveats Created: 10/Jul/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

Doc for array-map should mention its characteristics: preserves order of keys, linear O search so appropriate only for small maps, operations on array-maps return hash-maps.



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

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





[CLJ-148] Poor reporting of symbol conflicts when using (ns) Created: 10/Jul/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: errormsgs


 Description   

I have a module that includes pprint and my own utils.

When com.howard.lewisship.cascade.dom/write was changed from private to public I get the following error:

java.lang.IllegalStateException: write already refers to: #'clojure.contrib.pprint/write in namespace: com.howardlewisship.cascade.test-views (test_views.clj:0)

(ns com.howardlewisship.cascade.test-views ; line 15
(:use
(clojure.contrib test-is pprint duck-streams)
(app1 views fragments)
(com.howardlewisship.cascade config dom view-manager)
com.howardlewisship.cascade.internal.utils))

That line number is wrong but better yet, identifying the true conflict (com.howard.lewisship.cascade.dom/write) would be even more important.



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

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

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

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?

Comment by Jeff Rose [ 12/May/11 9:49 AM ]

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

Comment by Aaron Bedra [ 28/Jun/11 6:42 PM ]

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.





[CLJ-129] Add documentation to sorted-set-by detailing how the provided comparator may change set membership semantics Created: 18/Jun/09  Updated: 03/Sep/13

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

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


 Description   

To start, let's look at some simple default sorted-set behaviour (which uses PersistentHashMap via PersistentHashSet, and therefore uses equality/hashCode to determine identity):

user=> (sorted-set [1 2] [-5 10] [1 5])
#{[-5 10] [1 2] [1 5]}
</code></pre>

sorted-set-by uses PersistentTreeMap via PersistentTreeSet though, which implies that the comparator provided to sorted-set-by will be used to determine identity, and therefore member in the set.  This can lead to (IMO) non-intuitive behaviour:

<pre><code>
user=> (sorted-set-by #(> (first %) (first %2)) [1 2] [-5 10] [1 5])
#{[1 2] [-5 10]}

Notice that because the provided comparison fn determines that [1 2] and [1 5] have the same sort order, the latter value is considered identical to the former, and not included in the set. This behaviour could be very handy, but is also likely to cause confusion when what the user almost certainly wants is to maintain the membership semantics of the original set (e.g. relying upon equality/hashCode), but only modify the ordering.

(BTW, yes, I know there's far easier ways to get the order I'm indicating above over a set of vectors thanks to vectors being comparable via the compare fn. The examples are only meant to be illustrative. The same non-intuitive result would occur, with no easy fallback (like the 'compare' fn when working with vectors) when the members of the set are non-Comparable Java object, and the comparator provided to sorted-set-by is defining a sort over some values returned by method calls into those objects.)

I'd be happy to change the docs for sorted-set-by, but I suspect that there are others who could encapsulate what's going on here more correctly and more concisely than I.



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

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

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

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





[CLJ-19] GC Issue 15: JavaDoc for interfaces Created: 17/Jun/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: documentation


 Description   
Reported by richhickey, Dec 17, 2008
Add JavaDoc to those interfaces supported for public use - IFn,
IPersistentCollection etc.


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

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

Comment by Assembla Importer [ 24/Aug/10 6:44 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 Kevin Downey [ 17/Nov/12 8:05 PM ]

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





[CLJ-1045] Generalize/refactor implementation of PersistentVector/coll-fold Created: 18/Aug/12  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers

Attachments: Text File clj-1045-fold-by-halves-patch-v2.txt     Text File fold-by-halves.patch    
Patch: Code

 Description   

Vector currently contains a specialized implementation of the folding algorithm "split the collection in half until the pieces are small enough". The attached commit lifts out the general strategy so that it can be reused by other collection types amenable to splitting.

CLJ-993 depends on this patch, as it uses the new fold-by-halves function.



 Comments   
Comment by Andy Fingerhut [ 25/Jan/13 2:29 PM ]

clj-1045-fold-by-halves-patch-v2.txt dated Jan 25 2013 is identical to fold-by-halves.patch dated Aug 18 2012, except it updates one line of context changed by a recent commit to Clojure master.





[CLJ-153] Suggest adding set-precision! API to accompany with-precision Created: 12/Jul/09  Updated: 04/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: math


 Description   

Ticket #137 makes math-context</code> settable at the REPL. However, <code>math-context is not a public name in Clojure.

The related public function is with-precision</code> which works by pushing a new binding for <code>math-context</code>. This ticket suggests adding <code>set-precision!</code> to Clojure's public API. Its effect would be the same as <code>with-precision</code>, but accomplished by using <code>set!</code> on the current <code>math-context binding rather than by pushing a new binding.

Chouser suggests that we also add a doc string for math-context</code> noting that it is private and pointing the user to <code>with-precision</code> and <code>set-precision!. I agree.



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

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





[CLJ-272] load/ns/require/use overhaul Created: 18/Feb/10  Updated: 04/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: None


 Description   

Creating this ticket to describe various things people have wanted to change about how ns works:

Minimal needs

  1. there should be a primitive level of loading (presumably load) that just loads without question.
  2. the api should be unified across the ns and direct forms. No more keywords or quoting! So (use foo) not (use 'foo). This makes use et al macros, so there should also be new fn versions (maybe use*).

Other possibilities to discuss.

  1. Feature addressing the :like and :clone ideas from http://onclojure.com/2010/02/17/managing-namespaces/. I think I would prefer a single new option :clone which allows :only and :exclude features as subspecifiers.
  2. Convenience fn to unmap all names in a namespace?


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

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

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

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.

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

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

Comment by Stuart Sierra [ 12/Dec/10 4:08 PM ]

Further requests:

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

Comment by Paudi Moriarty [ 28/Feb/12 3:56 AM ]

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.





[CLJ-840] Add a way to access the current test var in :each fixtures for clojure.test Created: 21/Sep/11  Updated: 23/Nov/13

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

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

Attachments: File add-test-var.diff     File clj840-2.diff    
Patch: Code

 Description   

When looking at (log) output from tests written with clojure.test, I would like to be able to identify the output associated with each test. A mechanism to expose the current test var within an :each fixture would enable this.

One mechanism might be to bind a test-var var with the current test var before calling the each-fixture-fn in clojure.test/test-all-vars.



 Comments   
Comment by Stuart Sierra [ 07/Oct/11 4:33 PM ]

Or just pass the Var directly into the fixture. Vars are invokable.

Comment by Hugo Duncan [ 07/Oct/11 4:45 PM ]

I don't think that works, since the the function passed to the fixture is not the test var, but a function calling test-var on the test var.

Comment by Hugo Duncan [ 21/Oct/11 10:34 PM ]

Patch to add test-var

Comment by Stuart Sierra [ 25/Oct/11 6:04 PM ]

*testing-vars* already has this information, but it's not visible to the fixture functions because it gets bound inside test-var.

Perhaps the :each fixture functions should be called in test-var rather than in test-all-vars. (The namespace of a Var is available in its metadata.) But then we have to call join-fixtures inside test-var every time.

Comment by Stuart Sierra [ 25/Oct/11 6:26 PM ]

Try this patch: clj840-2.diff.

This makes *testing-vars* visible to :each fixture functions, which seems intuitively more correct.

BUT it slightly changes the behavior of test-var, which I'm less happy about.

Comment by Hugo Duncan [ 25/Oct/11 8:07 PM ]

Might it make sense to provide a function on top of testing-vars to return the current test-var?

Comment by Stuart Sierra [ 28/Oct/11 9:14 AM ]

No, that function is first

Comment by Hugo Duncan [ 28/Oct/11 11:31 AM ]

I agree with having the dynamic vars as part of the extension interface, but would have thought that having a function for use when writing tests would have been cleaner. Just my 2c.

Comment by Andy Fingerhut [ 23/Nov/13 12:42 AM ]

With a commit made on Nov 22, 2013, patch clj840-2.diff no longer applies cleanly to latest master. Updating it appears like it might be straightforward, but best for someone who knows this part of the code well to do so.





[CLJ-1272] Agent thread executors do not use the global uncaught exception handler Created: 01/Oct/13  Updated: 05/Feb/14

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

Type: Enhancement Priority: Major
Reporter: David Greenberg Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: agents


 Description   

If you use Thread.setDefaultUncaughtExceptionHandler to catch all application exceptions, and then throw an exception in a future, that exception will get swallowed up in deployment environments that don't watch stdout. It seems that the agent's executors ought to delegate to the global handler.

This issue bit us, in that we deploy and monitor our system only through its logs and metrics, and never actually saw that exceptions were being thrown in futures.






[CLJ-1180] defprotocol doesn't resolve tag classnames Created: 10/Mar/13  Updated: 05/Feb/14

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: protocols

Attachments: Text File 001-CLJ-1180.patch    
Patch: Code and Test

 Description   

defprotocol doesn't resolve tag classnames, this results in exceptions being thrown when the declared protocol uses as a tag an imported class that is not imported in the namespace that uses it.

user=> (import 'clojure.lang.ISeq)
clojure.lang.ISeq
user=> (defprotocol p (^ISeq f [_]))
p
user=> (ns x)
nil
x=> (defn x [y] (let [z (user/f y)] (inc z)))
CompilerException java.lang.IllegalArgumentException: Unable to resolve classname: ISeq, compiling:(NO_SOURCE_PATH:4:33)



 Comments   
Comment by Alex Miller [ 03/Sep/13 9:38 AM ]

Similer to CLJ-1232.





[CLJ-978] bean unable to handle non-public classes Created: 30/Apr/12  Updated: 05/Feb/14

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

Type: Enhancement Priority: Major
Reporter: Charles Duffy Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File clojure--bean-support-for-private-implementation-classes-v3.diff    
Patch: Code and Test

 Description   

Take the following Java as an example:

public interface IFoo {
  String getBar();
}

class FooImpl {
  String getBar() { return "bar"; }
}

As presently implemented, (bean my-foo) tries to invoke the following:

(. #<Method public java.lang.String FooImpl.getBar> (invoke my-foo nil))

However, as FooImpl is not public, this fails:

java.lang.IllegalAccessException: Class clojure.core$bean$fn__1827$fn__1828 can not access a member of class FooImpl with modifiers "public"
 at sun.reflect.Reflection.ensureMemberAccess (Reflection.java:65)
    java.lang.reflect.Method.invoke (Method.java:588)
    clojure.core$bean$fn__1827$fn__1828.invoke (core_proxy.clj:382)
    clojure.core$bean$v__1832.invoke (core_proxy.clj:388)
    clojure.core$bean$fn__1838$thisfn__1839$fn__1840.invoke (core_proxy.clj:406)
    clojure.lang.LazySeq.sval (LazySeq.java:42)
    clojure.lang.LazySeq.seq (LazySeq.java:60)
    clojure.lang.RT.seq (RT.java:473)

However, the same thing succeeds if we call #<Method public java.lang.String Foo.getBar> rather than #<Method public java.lang.String FooImpl.getBar>.



 Comments   
Comment by Charles Duffy [ 30/Apr/12 10:40 PM ]

Fix inaccurate documentation string

Comment by Charles Duffy [ 01/May/12 9:41 AM ]

Apache Commons Beanutils has their own implementation of this, at http://www.docjar.com/html/api/org/apache/commons/beanutils/MethodUtils.java.html#771 – notably, it tries to reflect a method with the given signature and catches the exception on failure, rather than iterating through the whole list. This may be a better approach – I'm unfamiliar with how the cost of exception handling compares with that of reflecting on the full method list of a class.

Comment by Charles Duffy [ 01/May/12 10:11 AM ]

Prior version of patch were missing new test suite files. Corrected.

Comment by Andy Fingerhut [ 04/May/12 2:48 AM ]

Thanks for the patches, Charles. Could you please create a patch in the desired format and attach that, and then remove the obsolete patches? Instructions for creating a patch are under the heading "Development" at this page: http://dev.clojure.org/display/design/JIRA+workflow

Instructions for removing patches are under the heading "Removing patches" on that same page.

Comment by Charles Duffy [ 06/May/12 2:59 PM ]

Added a patch created per documented process.

Comment by Gary Trakhman [ 04/Oct/12 6:44 PM ]

I found in my code that it's possible to get a NPE if there is no read-method, for instance on the http://docs.cascading.org/cascading/2.0/javadoc/cascading/flow/hadoop/HadoopFlow.html object which has a setCascade method but no getter. I fixed this in our code by inlining the is-zero-args check into the public-method definition and and-ing the whole thing with 'method' like the original 'bean' code, like so:

public-method (and method (zero? (alength (. method (getParameterTypes))))
(or (and (java.lang.reflect.Modifier/isPublic (. c (getModifiers)))
method)
(public-version-of-method method)))

Comment by Rich Hickey [ 29/Nov/12 10:01 AM ]

Charles, I think we should follow Apache BeanUtils on this. Exceptions not thrown are cheap. Ordinarily, exception for control flow are bad, but this is forced by bad design of reflection API.





[CLJ-1289] aset-* and aget perform poorly on multi-dimensional arrays even with type hints. Created: 01/Nov/13  Updated: 14/Feb/14

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

Type: Enhancement Priority: Major
Reporter: Michael O. Church Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: arrays, performance
Environment:

Clojure 1.5.1.

Dependencies: criterium


Attachments: Text File CLJ-1289-p1.patch    
Patch: Code
Approval: Triaged

 Description   

Here's a transcript of the behavior. I don't know for sure that reflection is being done, but the performance penalty (about 1300x) suggests it.

user=> (use 'criterium.core)
nil
user=> (def b (make-array Double/TYPE 1000 1000))
#'user/b
user=> (quick-bench (aget ^"[[D" b 304 175))
WARNING: Final GC required 3.5198021166354323 % of runtime
WARNING: Final GC required 29.172288684474303 % of runtime
Evaluation count : 63558 in 6 samples of 10593 calls.
             Execution time mean : 9.457308 µs
    Execution time std-deviation : 126.220954 ns
   Execution time lower quantile : 9.344450 µs ( 2.5%)
   Execution time upper quantile : 9.629202 µs (97.5%)
                   Overhead used : 2.477107 ns

One workaround is to use multiple agets.

user=> (quick-bench (aget ^"[D" (aget ^"[[D" b 304) 175))
WARNING: Final GC required 40.59820310542545 % of runtime
Evaluation count : 62135436 in 6 samples of 10355906 calls.
             Execution time mean : 6.999273 ns
    Execution time std-deviation : 0.112703 ns
   Execution time lower quantile : 6.817782 ns ( 2.5%)
   Execution time upper quantile : 7.113845 ns (97.5%)
                   Overhead used : 2.477107 ns

Cause: The inlined version only applies to arity 2, and otherwise it reflects.



 Comments   
Comment by Gary Fredericks [ 08/Dec/13 9:28 PM ]

A glance at the source makes it obvious that the hypothesis is correct – the inlined version only applies to arity 2, and otherwise it reflects.

I thought this would be as simple as converting the inline function to be variadic (using reduce), but after trying it I realized this is tricky as you have to generate the correct type hints for each step. E.g., given ^"[[D" the inline function needs to type-hint the intermediate result with ^"[D". This isn't difficult if we're just dealing with strings that begin with square brackets, but I don't know for sure that those are the only possibilities.

Comment by Yaron Peleg [ 13/Feb/14 4:44 AM ]

Bump. I just got bitten bad by this.

There are two seperate issues here:
1) (aget 2d-array-doubles 0 0 ) doesn't emit a reflection warning.
2) It seems like the compiler has enough information to avoid the reflective call here.

Note this gets exp. worse as number of dimensions grows, i.e (get doubles3d 0 0 0)
will be 1M slower, etc' Not true, unless you iterate over all elements. it's
simply n_dims*1000x per lookup.

Nasty surprise, especially considering you often go to primitive arrays for speed,
and a common use case is an inner loop(s) that iterate over arrays.

Comment by Gary Fredericks [ 13/Feb/14 7:08 AM ]

I can probably take a stab at this.

Comment by Gary Fredericks [ 13/Feb/14 8:34 PM ]

I think the reflection warning problem is pretty much impossible to solve without changing code elsewhere in the compiler, because the reflection done in aget is a different kind than normal clojure reflection – it's explicitly in the function body rather than emitted by the compiler. Since the compiler isn't emitting it, it doesn't reasonably know it's even there. So even if aget is fixed for other arities, you still won't get the warning when it's not inlined.

I can imagine some sort of metadata that you could put on a function telling the compiler that it will reflect if not inlined. Or maybe a more generic not-inlined warning?

The global scope of adding another compiler flag seems about balanced by the seriousness of array functions not being able to warn on reflection.

Comment by Gary Fredericks [ 13/Feb/14 8:52 PM ]

Attached CLJ-1289-p1.patch which simply inlines variadic calls to aget. It assumes that if it sees a :tag on the array arg that is a string beginning with [, it can assume that the return value from one call to aget can be tagged with the same string with the leading [ stripped off.

I'm not a jvm expert, but having read through the spec a little bit I think this is a reasonable assumption.

Comment by Alex Miller [ 14/Feb/14 3:34 PM ]

I think this probably is actually true, but a more official way to ask that question would be to get the array class and ask for Class.getComponentType() (and less janky than string munging).

Comment by Gary Fredericks [ 14/Feb/14 3:40 PM ]

How would you get the array class based on the :tag type hint?

Comment by Gary Fredericks [ 14/Feb/14 7:05 PM ]

I see (-> s (Class/forName) (.getComponentType) (.getName)) does the same thing – is that route preferred, or is there another one?





[CLJ-1079] Don't squash explicit :line and :column metadata in the MetaReader Created: 29/Sep/12  Updated: 03/Sep/13

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: reader

Attachments: File CLJ-1079.diff    
Patch: Code and Test

 Description   

I have been experimenting with using cljx to produce Clojure and ClojureScript source from a single file. This has gone well so far, with the exception that, due to the way the source transformation works, all of the linebreaks and other formatting is gone from the output. There is an option to include the original :line metadata in the output though, like so:

;;This file autogenerated from 
;;
;;  src/cljx/com/foo/hosty.cljx
;;
^{:line 1} (ns com.foo.hosty)
^{:line 3} (defn ^{:clj true} system-hash [x] ^{:line 5} (System/identityHashCode x))

(Hopefully, such hackery won't be necessary in the future with sjacket or something like it...)

Unfortunately, when read in using a LineNumberingPushbackReader, code like this has its :line metadata squashed by the line numbers coming from that. A REPL-friendly example would be:

=> (meta (read (clojure.lang.LineNumberingPushbackReader.
                 (java.io.StringReader. "^{:line 66} ()"))))
{:line 1}
=> (meta (read (java.io.PushbackReader.
                 (java.io.StringReader. "^{:line 66} ()"))))
{:line 66}

The latter seems more correct to me (and is equivalent to read-string).



 Comments   
Comment by Chas Emerick [ 29/Sep/12 7:07 PM ]

{{CLJ-1097.diff}} contains a fix for this issue, as well as a separate commit that eliminates a series of casts in order to improve readability in the area.

Comment by Andy Fingerhut [ 05/Oct/12 8:23 AM ]

Chas, your patch doesn't apply cleanly to latest Clojure master as of Oct 5 2012. I'm not sure, but I think some recent commits to Clojure on Oct 4 2012 caused that. I also take it as evidence of your awesomeness that you can write patches for tickets that haven't been filed yet

Comment by Chas Emerick [ 05/Oct/12 9:24 AM ]

"patches for tickets that haven't been filed yet?"

Anyway, tweaking up this patch is a small price to pay for having column meta. New {{CLJ-1097.diff}} patch attached, applies clean on master as of now. Otherwise same contents as in the original patch, except:

  • the same dynamic is also applied to :column metadata, now that it's available
  • the changes have been rebased into a single commit (including the elimination of the casts in MetaReader, which were becoming so numerous that the code was less readable than most
Comment by Nicola Mometto [ 05/Oct/12 9:39 AM ]

"patches for tickets that haven't been filed yet?"

He was referring to the fact that you uploaded "CLJ-1097.diff" while the ticket is #1079

Comment by Chas Emerick [ 05/Oct/12 9:42 AM ]

Oh, hah! Twice now, even! One more data point recommending my having slight dyslexia or somesuch. :-P

I've replaced the attached patch with one that is named properly to avoid any later confusion.

Comment by Chas Emerick [ 07/Oct/12 3:57 PM ]

Refreshed patch to apply cleanly to master after the recent off by one patch for :column metadata.

Comment by Stuart Halloway [ 19/Oct/12 3:12 PM ]

This feels backwards to me. If a special purpose tool wants to convey information via metadata, why does it use names that collide with those used by LispReader?

Comment by Chas Emerick [ 19/Oct/12 7:36 PM ]

The information being conveyed is the same :line and :column metadata conveyed by LispReader — in fact, that's where it comes from in the first place.

Kibit (and cljx) is essentially an out-of-band source transformation tool. Given an input like this:

(ns com.foo.hosty)

(defn ^:clj system-hash
  [x]
 (System/identityHashCode x))

(defn ^:cljs system-hash
  [x]
  (goog/getUid x))

…it produces two files, a .clj for Clojure, and a .cljs for ClojureScript. (The first code listing in the ticket description is the former.)

However, because there's no way to transform Clojure code/data without losing formatting, anything that depends on line/column numbers (stack traces, stepping debuggers) is significantly degraded. If LispReader were to defer to :line and :column metadata already available on the loaded forms (there when the two generated files are spit out with *print-meta* on), this would not be the case.

Comment by Andy Fingerhut [ 07/Feb/13 8:47 AM ]

clj-1079-patch-v2.txt dated Feb 7 2013 is identical to Chas's CLJ-1079.diff dated Oct 7 2012, except it applies cleanly to latest master. I believe the only difference is that some white space in the context lines is updated.

Comment by Andy Fingerhut [ 07/Feb/13 12:35 PM ]

Sorry for the noise. I've removed clj-1079-patch-v2.txt mentioned in the previous comment, because I learned that CLJ-1079.diff dated Oct 7 2012 applies cleanly to latest master and passes all tests if you use this command to apply it.

% git am --keep-cr -s --ignore-whitespace < CLJ-1079.diff

I will update the JIRA workflow page instructions for applying patches to mention this, too, because there are multiple patches that fail without --ignore-whitespace, but apply cleanly with that option. That will eliminate the need to update patches merely for whitespace changes.





[CLJ-366] Multiplatform command-line clojure launcher Created: 28/May/10  Updated: 17/Apr/14

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

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


 Description   

Clojure needs a lower barrier of entry, long java commands scare people away! We need a script that conveniently launches a clojure repl or executes clojure files, much like the ruby/python/perl/other-favorite-interpreted-language behavior.

NOTES:

From Russ Olson (regarding Dejure/Dejour):

  • I just fixed a bunch of bugs in the script, so make sure you get the latest from download from: http://github.com/russolsen/dejour
  • After looking at jruby, scala, and groovy, it seems that the only way to do this right on windows is to write a C or C++ program and have a .exe.


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

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

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

stu said: Updating tickets (#370, #366, #374)

Comment by Aaron Bedra [ 10/Dec/10 10:13 AM ]

Design page is at http://dev.clojure.org/display/design/CLJ+Launcher and should be the basis for all future discussion

Comment by Kevin Downey [ 17/Apr/14 10:22 PM ]

is the priority on this, as the ticket says, Major?

the last commit on https://github.com/russolsen/dejour was 2 years ago.
the last edit to the wikipage was 3 years ago.





[CLJ-450] Add default predicate argument to filter, every?, take-while Created: 01/Oct/10  Updated: 17/Apr/14

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

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

Attachments: Text File clj-450-add-default-pred-arg-to-core-fns-patch.txt     Text File clojure-default-every-argument-v1.patch    

 Description   

Some seq processing functions that take predicates could be improved by the addition of a default value of identity for the predicate argument.

This has been discussed on the mailing list, and people seem favorable:
http://groups.google.com/group/clojure/browse_thread/thread/600559b7ee261908/3bc5d144ac54854e?lnk=gst&q=filter+identity#3bc5d144ac54854e
http://groups.google.com/group/clojure-dev/browse_thread/thread/0a9b5750dd7ec4ca

I can put together a patch.



 Comments   
Comment by Assembla Importer [ 01/Oct/10 4:39 PM ]

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

Comment by Jason Orendorff [ 13/Mar/12 2:51 PM ]

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.

Comment by Jason Orendorff [ 13/Mar/12 4:57 PM ]

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

Comment by Andy Fingerhut [ 26/Apr/12 7:51 PM ]

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.

Comment by Jason Orendorff [ 27/Apr/12 10:35 AM ]

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?

Comment by Andy Fingerhut [ 27/Apr/12 11:38 AM ]

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.

Comment by Kevin Downey [ 17/Apr/14 10:33 PM ]

it doesn't seem productive to dance around: "I'll send in a ca, if you agree to take my patch" "We might take your patch but first send in a ca"

if there is no signed ca I think the patch should be removed from jira

Comment by Alex Miller [ 17/Apr/14 10:39 PM ]

Generally, I do not look at patches from people that have not signed a CA in case I need to write a patch later that is "clean".





[CLJ-127] DynamicClassLoader's call to ClassLoader.getSystemClassLoader is prohibited in some environments Created: 18/Jun/09  Updated: 18/Apr/14

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: classloader


 Description   

Currently, clojure.lang.DynamicClassLoader's constructor has the
following call to super():

super(EMPTY_URLS,
      (Thread.currentThread().getContextClassLoader() == null ||
        Thread.currentThread().getContextClassLoader() == ClassLoader.getSystemClassLoader()) ?
          Compiler.class.getClassLoader() : Thread.currentThread().getContextClassLoader());

That call to ClassLoader.getSystemClassLoader() is forbidden by Google
AppEngine's security policies. That restricts you from being able to
load any resources from the classpath that haven't been AOT-compiled.
I've verified that just removing that removing the " ||
Thread.currentThread().getContextClassLoader() ==
ClassLoader.getSystemClassLoader()" does in fact result in something
that works in GAE (as far as my needs go). Unfortunately, I'm not sure
whether that breaks anything, which, presumably, it does.



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

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

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

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.

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

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

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

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

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

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

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

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

Comment by Kevin Downey [ 18/Apr/14 12:05 AM ]

hard to say if this is still an issue, but I have been able to run clojure code on GAE in the past





[CLJ-1280] Create reusable exception that can carry file/line/col info Created: 18/Oct/13  Updated: 18/Apr/14

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

This concept already exists in multiple places in Clojure - Compiler$CompilerException and the Exception classes buried in EdnReader and LispReader. It would also be useful in other places where IllegalArgument or other other exceptions are thrown.

For example, this protocol exception throws an IllegalArgumentException and could transmit the file, line, and column info at the location of the error but it seems weird to use any of the existing exceptions for this purpose.

(defprotocol Bar (m [this]) (m [this arg]))


 Comments   
Comment by Kevin Downey [ 18/Apr/14 12:09 AM ]

seems like ExceptionInfo can do this





[CLJ-1013] Clojure's classloader cannot handle out-of-order loading Created: 13/Jun/12  Updated: 18/Apr/14

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

Type: Defect Priority: Major
Reporter: Edward Z. Yang Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

Here is a minimal test-case:

import java.io.IOException;

import clojure.lang.PersistentTreeMap;
import clojure.lang.RT;

public class TestClass {

static Class y = RT.class;
//static PersistentTreeMap x = PersistentTreeMap.EMPTY;

/**

  • @param args
  • @throws ClassNotFoundException
  • @throws IOException
    */
    public static void main(String[] args) throws IOException, ClassNotFoundException { PersistentTreeMap x = PersistentTreeMap.EMPTY; }

}

This results in the exception:

Exception in thread "main" java.lang.ExceptionInInitializerError
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:247)
at clojure.lang.RT.loadClassForName(RT.java:2056)
at clojure.lang.RT.load(RT.java:419)
at clojure.lang.RT.load(RT.java:400)
at clojure.lang.RT.doInit(RT.java:436)
at clojure.lang.RT.<clinit>(RT.java:318)
at clojure.lang.PersistentTreeMap.<init>(PersistentTreeMap.java:45)
at clojure.lang.PersistentTreeMap.<clinit>(PersistentTreeMap.java:32)
at TestClass.main(TestClass.java:19)
Caused by: java.lang.NullPointerException
at clojure.lang.APersistentSet.contains(APersistentSet.java:33)
at clojure.lang.RT.contains(RT.java:700)
at clojure.core$contains_QMARK_.invoke(core.clj:1386)
at clojure.core$load_lib.doInvoke(core.clj:5255)
at clojure.lang.RestFn.applyTo(RestFn.java:142)
at clojure.core$apply.invoke(core.clj:603)
at clojure.core$load_libs.doInvoke(core.clj:5298)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:603)
at clojure.core$require.doInvoke(core.clj:5381)
at clojure.lang.RestFn.invoke(RestFn.java:408)
at clojure.core__init.load(Unknown Source)
at clojure.core__init.<clinit>(Unknown Source)
... 10 more

The crux of the issue appears Clojure's classloader doesn't understand how to handle out-of-order classloading.



 Comments   
Comment by Kevin Downey [ 18/Apr/14 12:31 AM ]

exception still happens with clojure 1.6





[CLJ-1431] Switch from MurmurHash3 to SipHash to prevent DoS collision attack (hash flooding) Created: 25/May/14  Updated: 26/May/14

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

Type: Enhancement Priority: Major
Reporter: James Thornton Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: security


 Description   

Clojure is using Murmur3 throughout:
https://github.com/clojure/clojure/commit/dff9600387b962f16fc78e6477e10e34651fd366

DJB, Jean-Philippe Aumasson, and Martin Boßlet have shown that Murmur3 is not resilient against hash collision attacks:
http://www.ocert.org/advisories/ocert-2012-001.html
https://131002.net/siphash/

"Hash-flooding DoS reloaded: attacks and defenses" talk by DJB, Jean-Philippe Aumasson, and Martin Boßlet
http://media.ccc.de/browse/congress/2012/29c3-5152-en-hashflooding_dos_reloaded_h264.html

"Breaking Murmur: Hash-flooding DoS Reloaded"
http://emboss.github.io/blog/2012/12/14/breaking-murmur-hash-flooding-dos-reloaded/

Python, Ruby, JRuby, Haskell, Rust, Perl, Redis... have all switched to SipHash
https://en.wikipedia.org/wiki/SipHash

Last year Google dropped CityHash from Guava and replaced it with SipHash
https://code.google.com/p/guava-libraries/issues/detail?id=1232

SipHash Guava Implementation
https://code.google.com/p/guava-libraries/source/browse/guava/src/com/google/common/hash/SipHashFunction.java

SipHash Java reference implementation
https://github.com/emboss/siphash-java/blob/master/src/main/java/com/github/emboss/siphash/SipHash.java



 Comments   
Comment by Alex Miller [ 26/May/14 12:56 AM ]

Thanks, we've talked about this issue and some possible things we could do, but didn't have a ticket for it yet.

Comment by Alex Miller [ 26/May/14 1:08 AM ]

While the Java 7 approach relied on (attempting) to properly seed hash maps with string hash codes, that was all dropped in Java 8, which addressed DoS collision hash attacks by instead improving the data structure to switch from linear collisions to a red/black tree (log-time) for collisions. It's possible a similar approach could work in Clojure as well.

One workaround that could be used now is to wrap map keys in a custom type that implements IHashEq and implements an alternate hash function.





[CLJ-1096] Make destructuring emit direct keyword lookups Created: 29/Oct/12  Updated: 06/Jun/14

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

Type: Enhancement Priority: Major
Reporter: Christophe Grand Assignee: Christophe Grand
Resolution: Unresolved Votes: 2
Labels: performance

Attachments: File desctructure-keyword-lookup.diff     File inline-get-keyword.diff    
Patch: Code
Approval: Triaged

 Description   

Currently associative destructuring emits calls to get. The attached patch modify desctruture to emit direct keyword lookups when possible.

Approved here https://groups.google.com/d/msg/clojure-dev/MaYcHQck8VA/nauMus4mzPgJ



 Comments   
Comment by Christophe Grand [ 04/Sep/13 3:40 AM ]

Rethinking about this patch now, it may be too specific: get's inline expansion should be modified when the key is a literal keyword.

Comment by Christophe Grand [ 04/Sep/13 3:41 AM ]

More generic patch (inline-get-keyword.diff): all get calls with literal keywords as keys are inlined to direct keyword lookup.

Comment by John Hume [ 19/May/14 1:14 PM ]

Is this only stalled out of lack of interest?

Comment by Andy Fingerhut [ 19/May/14 6:13 PM ]

There are currently about 50 tickets "triaged", i.e. marked for Rich to look at and decide whether they are things he is interested in seeing a patch for, and another 25 or so that were triaged and he has "vetted" them, and they are in various stages of having patches written for them, screened, etc. That doesn't mean anything for this ticket in particular – just wanted to make it clear that there are a bunch of other tickets that are getting some attention, and a bunch of others that are not.

What gets triaged depends somewhat upon how severe the issue appears. You can vote on the ticket, and try to persuade others to do so as well, if they think this would enhance the performance of some commonly-written types of Clojure code. You could also consider doing some benchmarking with & without these patches to see how much performance they can gain.





[CLJ-825] Protocol implementation inconsistencies when overloading arity Created: 08/Aug/11  Updated: 10/Jun/14

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

Type: Enhancement Priority: Major
Reporter: Carl Lerche Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: protocols
Environment:

All


Attachments: Text File clj-825-1.patch     File scribbles.clj    
Patch: Code and Test
Approval: Triaged

 Description   

The forms required for implementing arity-overloaded protocol methods are inconsistent between the "extend-*" macros and "defrecord".

The "extend" family of macros requires overloaded method definitions to follow the form used by defn:

(method ([arg1] ...) ([arg1 arg2] ...))

However, "defrecord" requires implementations to be defined separately:

(method [arg1] ...)
(method [arg1 arg2] ...)

Furthermore, the error modes if you get it wrong are unhelpful.

If you use the "defrecord" form with "extend-*", it evals successfully, but later definitions silently overwrite lexically previous definitions.

If you use the "extend-*" form with "defrecord", it gives a cryptic error about "unsupported binding form" on the body of the method.

This is not the same issue as CLJ-1056: That pertains to the syntax for declaring a protocol, this problem is with the syntax for implementing a protocol.

(defprotocol MyProtocol
  (mymethod
    [this arg]
    [this arg optional-arg]))

(extend-protocol MyProtocol
  Object
  (mymethod
    ([this arg] :one-arg)
    ([this arg optional-arg] :two-args)))

;; BAD! Blows up with "Unsupported binding form: :one-arg"
(defrecord MyRecord []
  MyProtocol
  (mymethod
    ([this arg] :one-arg)
    ([this arg optional-arg] :two-args)))

;; Works...
(defrecord MyRecord []
  MyProtocol
  (mymethod [this arg] :one-arg)
  (mymethod [this arg optional-arg] :two-args))

;; Evals...
(extend-protocol MyProtocol
  Object
  (mymethod [this arg] :one-arg)
  (mymethod [this arg optional-arg] :two-args))

;; But then... Error! "Wrong number of args"
(mymethod :obj :arg)

;; 2-arg version is invokable...
(mymethod :obj :arg1 :arg2)


 Comments   
Comment by Paavo Parkkinen [ 17/Nov/13 6:02 AM ]

Attached a patch for this.

For defrecord, I check which style is used for defining methods, and transform into the original style if the new style is used. For the check I do what I believe defn does, which is (vector? (first fdecl)).

For extend-*, I skip the checking, and just transform everything into the same format.

Tests included for both.

All tests pass.

Comment by Rich Hickey [ 10/Jun/14 11:00 AM ]

What the proposal?





[CLJ-1221] Should repackage jsr166 and include known version with Clojure Created: 20/Jun/13  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Alex Miller
Resolution: Unresolved Votes: 0
Labels: reducers


 Description   

Clojure 1.5 reducers work with either the JDK version of forkjoin (JDK 1.7+) or with an external jsr166 jar. This causes complexity for users and complexity in the build to deal with the two options.

jsr166 code is public domain and it is common for other projects to repackage the handful of files and ship it with the project (similar to what we do with asm). This would allow us just use a known existing version of jsr166 across all jdks and we could get rid of the custom build wrangling we introduced in Clojure 1.5.

jsr166y is compatible with JDK 1.6+ and is the version that (for example) Scala currently repackages. That's the best choice for JDK 1.6 and 1.7. In JDK 1.8, the best choice will (temporarily) be the built-in version in java.util.concurrent which tracks jsr166e but then as soon as there are updates will become jsr166e. Many fork/join fixes are ported to both y and e right now.

Some choices here for JDK 1.8:

  • go for maximal compatibility just use repackaged jsr166y regardless of JDK (simplest)
  • check for jdk version # and use java.util.concurrent instead
  • check for jdk version # and repackage jsr166e and use it instead

Not sure yet which of these is best choice right now.






[CLJ-1059] PersistentQueue doesn't implement java.util.List, causing nontransitive equality Created: 03/Sep/12  Updated: 11/Aug/14

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

Type: Defect Priority: Major
Reporter: Philip Potter Assignee: Philip Potter
Resolution: Unresolved Votes: 1
Labels: queue

Attachments: File 001-clj-1059-make-persistentqueue-implement-list.diff     File 002-clj-1059-asequential-rebased-to-cached-hasheq.diff    
Patch: Code and Test

 Description   

PersistentQueue implements Sequential but doesn't implement java.util.List. Lists form an equality partition, as do Sequentials. This means that you can end up with nontransitive equality:

(def q (conj clojure.lang.PersistentQueue/EMPTY 1 2 3))
;=> #user/q
(def al (doto (java.util.ArrayList.) (.add 1) (.add 2) (.add 3)))
;=> #user/al
(def v [1 2 3])
;=> #user/v
(= al v)
;=> true
(= v q)
;=> true
(not= al q)
;=> true

This happens because PersistentQueue is a Sequential but not a List, ArrayList is a List but not a Sequential, and PersistentVector is both.



 Comments   
Comment by Philip Potter [ 15/Sep/12 3:41 AM ]

Whoops, according to http://dev.clojure.org/display/design/JIRA+workflow I should have emailed clojure-dev before filing this ticket. Here is the discussion:

https://groups.google.com/d/topic/clojure-dev/ME3-Ke-RbNk/discussion

Comment by Philip Potter [ 15/Sep/12 2:37 PM ]

Attached 001-make-PersistentQueue-implement-List.diff, 15/Sep/12

Note that this patch has a minor conflict with the one I added to CLJ-1070, because both add an extra interface to PersistentQueue - List in this case, IHashEq in CLJ-1070.

Comment by Chouser [ 18/Sep/12 1:04 AM ]

Philip, patch looks pretty good – thanks for doing this. A couple notes:

This is only my opinion, but I prefer imports be listed without wildcards, even if it means an extra couple lines at the top of a .java file.

I noticed the "List stuff" code is a copy of what's in ASeq and EmptyList. I suppose this is copied because EmptyList and PersistentQueue extend Obj and therefore can't extend ASeq. Is this the only reason? It seems a shame to duplicate these method definitions, but I don't know of a better solution, do you?

It would also be nice if the test check a couple of the List methods you've implemented.

Comment by Chouser [ 18/Sep/12 1:08 AM ]

oh, also "git am" refused to apply the patch, but I'm not sure why. "patch -p 1" worked perfectly.

Comment by Philip Potter [ 18/Sep/12 1:19 AM ]

did you use the --keep-cr option to git am?

I struggled to know whether I should be adding CRs or not to line endings, because the files I was editing weren't consistent in their usage. If you open them in emacs, half the lines have ^M at the end.

Comment by Philip Potter [ 18/Sep/12 1:21 AM ]

Will submit another patch, with the import changed. I'll have a think about the list implementation and see what ideas I can come up with.

Comment by Philip Potter [ 18/Sep/12 3:17 PM ]

Attached 002-make-PersistentQueue-implement-Asequential.diff

This patch is an alternative to 001-make-PersistentQueue-implement-List.diff

So I took on board what you said about ASeq, but it didn't feel right making PersistentQueue directly implement ISeq, somehow.

So I split ASeq into two parts – ASequential, which implements j.u.{Collection,List} and manages List-equality and hashcodes; and ASeq, which... doesn't seem to be doing much anymore, to be honest.

As a bonus, this patch fixes CLJ-1070 too, so I went and added the tests from that ticket in to demonstrate this fact. It also tidies up PersistentQueue by removing all equals/hashcCode stuff and all Collection stuff.

(It turns out that because ASeq was already implementing Obj, the fact that PersistentQueue was implementing Obj was no barrier to using it.)

Would appreciate comments on this approach, and how it differs from the previous patch here and the patch on CLJ-1070.

Comment by Philip Potter [ 18/Sep/12 3:44 PM ]

Looking at EmptyList's implementation of List, it is a duplicate of the others, but it shouldn't be. I think its implementation of indexOf is the biggest culprit - it should just be 'return -1;' but it has a great big for loop! But this is beyond the scope of this ticket, so I won't patch that here.

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

Philip, now that the patch for CLJ-1070 has been applied, these patches no longer apply cleanly. Would you be willing to update them? If so, please remove the obsolete patches.

Comment by Philip Potter [ 22/Oct/12 5:10 AM ]

Andy, thanks so much for your efforts to make people aware of these things. I will indeed submit new patches, hopefully later this week.

Comment by Philip Potter [ 03/Nov/12 12:23 PM ]

Replaced existing patches with new ones which apply cleanly to master.

There are two patches:

001-clj-1059-make-persistentqueue-implement-list.diff

This fixes equality by making PersistentQueue implement List directly. I also took the opportunity to remove the wildcard import and to add tests for the List methods, as compared with the previous version of the patch.

002-clj-1059-asequential.diff

This fixes equality by creating a new abstract class ASequential, and making PersistentQueue extend this.

My preferred solution is still the ASequential patch, but I'm leaving both here for comparison.

Comment by Timothy Baldridge [ 30/Nov/12 3:37 PM ]

Vetting.

Comment by Andy Fingerhut [ 11/Dec/12 12:50 PM ]

Philip, this time I think it was patches that were committed for CLJ-1000 that make your patch 002-clj-1059-asequential.diff not apply cleanly. I often fix up stale patches where the change is straightforward and mechanical, but in this case you are moving some methods that CLJ-1000's patch changed the implementation of, so it would be best if someone figured out a way to update this patch in a way that doesn't clobber the CLJ-1000 changes.

Comment by Philip Potter [ 11/Dec/12 1:57 PM ]

Thanks Andy. Submitted a new patch, 002-clj-1059-asequential-rebased-to-cached-hasheq.diff, which supersedes 002-clj-1059-asequential.diff.

The patch 001-clj-1059-make-persistentqueue-implement-list.diff still applies cleanly, and is still an alternative to 002-clj-1059-asequential-rebased-to-cached-hasheq.diff.

Comment by Andy Fingerhut [ 30/Jan/14 4:50 PM ]

With the commits to Clojure master made in the week leading up to Jan 30 2014, particularly changes to hasheq, patch 002-clj-1059-asequential-rebased-to-cached-hasheq.diff no longer applies cleanly.

Patch 001-clj-1059-make-persistentqueue-implement-list.diff still does.

Comment by Andy Fingerhut [ 31/Mar/14 5:33 PM ]

This issue was run into again and a duplicate ticket CLJ-1374 created – later closed as a duplicate of this one. Just wanted to record that this issue is being hit by others besides those originally reporting it.

Comment by Andy Fingerhut [ 11/Aug/14 1:37 AM ]

One or more commits made to Clojure master between Aug 1 2014 and Aug 10 2014 conflict with the patch 001-clj-1059-make-persistentqueue-implement-list.diff, and it no longer applies cleanly.





[CLJ-1504] Add :inline to most core predicates Created: 15/Aug/14  Updated: 15/Aug/14

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

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

Attachments: Text File 0001-add-inline-to-some-core-predicates.patch    
Patch: Code

 Description   

This will allow instance? predicates calls to be emitted using the instanceof JVM bytecode and will also allow tools like core.typed or tools.analyzer.jvm to infer the type of a var/local on a per branch basis without having to special-case all the core predicates.



 Comments   
Comment by Jozef Wagner [ 15/Aug/14 1:32 PM ]

Related ticket CLJ-1227 and related quote from Alex:

definline is considered to be an experimental feature and Rich would like to discourage its use as the hope is to remove it in the future. The desired replacement is something like common lisp compiler macros that could allow the compiler to detect special situations and optimize the result but leave behind a function invocation for the case where no special behavior is available.

Comment by Nicola Mometto [ 15/Aug/14 1:42 PM ]

This patch uses "manual" :inline metadata on functions, it's used by many other core functions (like +,- et), not definline so Rich's comment doesn't apply.





[CLJ-1046] Drop-while as a reducer Created: 18/Aug/12  Updated: 03/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers

Attachments: Text File drop-while-reducer.patch    
Patch: Code and Test

 Description   

Implement drop-while as a reducer. Follows the same atom-based strategy as drop and take.

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 before CLJ-992 and CLJ-993.



 Comments   
Comment by Andy Fingerhut [ 03/Sep/14 8:11 PM ]

Patch drop-while-reducer.patch dated Aug 18 2012 no longer applies cleanly to latest master after some commits were made to Clojure on Sep 3, 2014.

I have not checked whether this patch is straightforward to update, nor do I know if there is interest in an updated patch, or whether transducers are the preferred way to go over reducers, and thus all reducers-related tickets will be closed. See the section "Updating stale patches" at http://dev.clojure.org/display/community/Developing+Patches for suggestions on how to update patches.





[CLJ-1520] assoc-in with empty key path assoc-es to nil Created: 05/Sep/14  Updated: 05/Sep/14

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

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


 Description   
(assoc-in {} [] 1) ;=> {nil 1}

This should probably throw an exception.

CLJ-373 has a patch (CLJ-373-nested-ops.patch) which fixes this (by throwing an exception on empty key paths), the related broken behavior of update-in, and documents empty key path behavior in get-in et al. I can pull just the assoc-in stuff out of that into a separate patch, but I am really hoping that all the issues in the patch addresses are resolved at once, I.e.:

(get-in {} [] :notfound) ;=> {} ; ok
(get-in {nil 1} [] :notfound) ;=> {nil 1} ; ok
(assoc-in {} [] 1) ;=> {nil 1} ; wat?
(assoc-in {nil 0} [] 1) ;=> {nil 1} ; wat?
(update-in {} [] identity) ;=> {nil nil} ; wat?
(update-in {nil 0} [] inc) ;=> {nil 1} ; wat?





[CLJ-200] Extend cond to support inline let, much like for Created: 18/Oct/09  Updated: 22/Oct/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Mark Engelberg
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File clj-200-cond-let-clauses-fixed-test-v2.diff     Text File clj-200-cond-let-clauses-fixed-test-v2-patch.txt    
Patch: Code and Test

 Description   

I find it occasionally very useful to do a few tests in a cond, then introduce some new symbols (for both clarity and efficiency) that can be referenced in later tests (or matching expressions). This parallels similar functionality inside the for macro, where the :let keyword is matched against a vector of symbol bindings and forms an implicit let around the remainder of the comprehension.

I'll be adding a patch for this shortly.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

hlship said: Trickier than I thought because cond is really wired into other fundamentals, like let.

Comment by Assembla Importer [ 24/Aug/10 1:51 PM ]

cgrand said: Howard, what do you think of http://gist.github.com/432712 ?

Comment by Mark Engelberg [ 23/Nov/12 2:33 AM ]

Patch cond-let-clauses.diff on 23/Nov/12 adds inline :let clauses to cond, implementing CLJ-200. The code is based off of code by cgrand, with some tweaks so the implementation only relies on constructs defined earlier in core.clj, since when cond is defined, things aren't yet fully bootstrapped. Also added a test to control.clj.

Comment by Christophe Grand [ 23/Nov/12 3:06 AM ]

Some comments: the docstring is missing, I believe you don't have to modify the original cond (except the docstring maybe), just redefine it later on once most of the language is defined – a bit like what is done for let for example.

There is still the unlikely eventuality that some code uses :let as :else. What about shipping a cond which complains on keywords (in test position) other than :else?

Comment by Mark Engelberg [ 23/Nov/12 3:47 AM ]

cond-let-clauses-with-docstring.diff contains the same patches as cond-let-clauses, but includes the original docstring for cond along with an additional sentence about the :let bindings.

Comment by Mark Engelberg [ 23/Nov/12 3:54 AM ]

Cgrand, I did see your example of redefining cond after most of the language is defined, but since I was able to figure out how to do it in the proper place, that makes the :let bindings available for users of cond downstream and avoids any unforeseen complications that might come from rebinding.

As for your other point, I think it is highly improbable that someone would have used :let in the :else position. However I can imagine someone intentionally using something like :true or :default. I think the idea of warning for other keywords is actually more likely to cause complications than the unlikely problem it is meant to solve.

I did resubmit the patch with the docstring restored. Thanks for pointing out that problem. I'm excited about this patch – I use :let bindings within the cond in my own code all the time. Thanks again for the blog post that started me on that path.

Comment by Christophe Grand [ 23/Nov/12 4:13 AM ]

True, it's :unlikely for :let to happen.
However once :let is officially blessed, it may be better to provision for future other "special" keywords and thus to warn on "unsupported" keywords. Plus it will help out-of-order typists (like myself) to catch earlier a :elt instead of a :let
This is only my point of view. Thanks for trying to get :let in cond supported.

Comment by Andy Fingerhut [ 29/Nov/12 8:46 PM ]

Mark, could you remove the obsolete earlier patch now that you have added the one with the doc string? Instructions for removing patches are under the heading "Removing Patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Comment by Mark Engelberg [ 29/Nov/12 10:50 PM ]

Done.

Comment by Andy Fingerhut [ 30/Nov/12 1:24 AM ]

I haven't figured out what is going wrong yet. I can apply the patch cond-let-clauses-with-docstring.diff to the latest Clojure master just fine. I can do "ant jar" and it will build a jar. When I do "ant", it fails with the new test for cond with :let, throwing a StackOverflowException. I can enter that same form into the REPL and it evaluates just as the test says it should. I can comment out that new test and all of the rest pass. But the new test doesn't pass when inside of the control.clj file. Anyone know why?

Comment by Christophe Grand [ 30/Nov/12 4:54 AM ]

It's because of the brutal replacement performed by test/are: the placeholders for this are form are x and y but in Mark's test there are used as local names and are tries to substitute them recursively...
If one changes the local names to a and b for example it works.

Comment by Mark Engelberg [ 02/Dec/12 8:20 AM ]

cond-let-clauses-fixed-test.diff on 02/Dec/12 contains the same patch, but with the x,y locals in the test case changed to a,b so that it works properly in the are clause which uses x and y.

Comment by Mark Engelberg [ 02/Dec/12 8:27 AM ]

On Windows, I can't get Clojure's test suite task to work, either via ant or maven, which has made it difficult for me to verify the part of the patch that applies to the test suite works as expected; I had tested it as best I could in the REPL, using a version of Clojure built with the patch applied, but using this process, I missed the subtle interaction between are and the locals in the test case. Sorry about that. If someone can double-check that the test suite task now works with the newest patch, that would be great, and then I'll go ahead and remove the obsoleted patch. Thanks.

Comment by Andy Fingerhut [ 02/Dec/12 6:29 PM ]

clj-200-cond-let-clauses-fixed-test-v2-patch.txt dated Dec 2 2012 is identical to Mark Engelberg's cond-let-clauses-fixed-test.diff of the same date, except it applies cleanly to the latest Clojure master.

I've verified that it compiles and passes all tests with latest Clojure master as of this date.

Mark, I've made sure to keep your name in the patch, since you wrote it. You should be able to remove your two attachments now, so the screener won't be confused which patch should be examined.

Comment by Andy Fingerhut [ 02/Dec/12 6:31 PM ]

Mark, besides general issues with Windows not being used much (or maybe not at all?) by Clojure developers, there is the issue right now filed as CLJ-1076 that not all tests pass when run on Windows due to CR-LF line ending differences that cause several Clojure tests to fail, regardless of whether you use ant or maven to run them.

Comment by Andy Fingerhut [ 22/Oct/13 8:01 PM ]

clj-200-cond-let-clauses-fixed-test-v2.diff is identical to earlier patch clj-200-cond-let-clauses-fixed-test-v2-patch.txt, except it removes unnecessarily trailing whitespace that causes warnings when applying the patch, and with .diff suffix for easier diff-style viewing in some editors.





[CLJ-1277] Speed up printing of time instants by adding type hints Created: 10/Oct/13  Updated: 03/Oct/14

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

Type: Enhancement Priority: Major
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: ft, performance

Attachments: Text File clj-1277-1.txt    
Patch: Code
Approval: Triaged

 Description   

There are several occurrences of reflection in instant.clj that slow down the printing of time instants.

Clojure Google group conversation link: https://mail.google.com/mail/u/0/?shva=1#label/clojure/1419e1e6f6cc5b3d

The addition of a few type hints is enough to speed the printing of time instants by a factor of about 3 to 4.5, in a few small benchmarks.

Patch: clj-1277-1.txt
Screened by: Alex Miller



 Comments   
Comment by Andy Fingerhut [ 10/Oct/13 12:07 AM ]

Patch clj-1277-1.txt adds 4 type hints that eliminate all reflection occurrences in source file instant.clj. Benchmarks show that it speeds up printing of java.util.Date and java.sql.Timestamp objects by a factor of about 3 to 4.5.

Latest Clojure master as of Oct 9 2013:

user=> (time (let [d (java.util.Date.)] (dotimes [i 3000000] (pr-str d))))
"Elapsed time: 24094.282 msecs"
user=> (import 'java.sql.Timestamp)
user=> (time (let [d (java.sql.Timestamp. 1300000000000)] (dotimes [i 2000000] (pr-str d))))
"Elapsed time: 20856.957 msecs"

That version of Clojure plus the patch clj-1277-1.txt:

user=> (time (let [d (java.util.Date.)] (dotimes [i 3000000] (pr-str d))))
"Elapsed time: 5085.847 msecs"
user=> (time (let [d (java.sql.Timestamp. 1300000000000)] (dotimes [i 2000000] (pr-str d))))
"Elapsed time: 7582.233 msecs"

Comment by Alexander Kiel [ 10/Oct/13 4:54 AM ]

Thanks for the patch Andy. But I don't like the (set! warn-on-reflection true). I think its better to use it only in the dev profile in leiningen. Not in real production code.

Comment by Andy Fingerhut [ 10/Oct/13 4:06 PM ]

Alexander, Leiningen is not used for building Clojure itself. The two supported choices are Maven and ant. Several Clojure source files, e.g. core/protocols.clj and core/reducers.clj, set warn-on-reflection to true, I believe so that if code is changed in such a way as to introduce a warning, it will be caught more quickly.

If the screeners or Rich think it is inappropriate, it is easy enough to remove.

Comment by Alex Miller [ 10/Oct/13 4:48 PM ]

Setting that is not uncommon in core clojure code and seems fine to me here.





[CLJ-1545] Add unchecked-divide, unchecked-remainder Created: 02/Oct/14  Updated: 06/Oct/14

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

Type: Enhancement Priority: Major
Reporter: Andy Fingerhut Assignee: Colin Taylor
Resolution: Unresolved Votes: 0
Labels: math, newbie

Attachments: File CLJ-1545-2.diff     File CLJ-1545.diff    
Patch: Code and Test
Approval: Triaged

 Description   

This appears like it might be an oversight that these are missing. There are unchecked-divide-int and unchecked-remainder-int functions, but not equivalents for longs, even though there are equivalents for longs for every other unchecked operation. The JVM has bytecodes for long division and remainder.

The Clojure documentation in the section "Support for Java Primitives" on page http://clojure.org/java_interop has links for unchecked-divide and unchecked-remainder, but since they don't exist in Clojure, the API link targets don't exist.

It seems like a good idea to either add these to Clojure, or remove them from the documentation.



 Comments   
Comment by Colin Taylor [ 03/Oct/14 6:17 PM ]

Having a go at this.

Comment by Colin Taylor [ 04/Oct/14 6:02 AM ]
  • Added tests for unchecked-divide-int and unchecked-remainder-int too.
  • Unchecked fns only support binary arity and will throw CompilerException(ArityException)s where checked will not.
  • Is there any value to (int,long) (long,int) overrides for java interop cases e.g. using java collections from Clojure in high perf code?
Comment by Alex Miller [ 04/Oct/14 9:13 AM ]

Thanks for taking this on Colin!

1) When I apply the patch (git apply CLJ-1545.diff), I get a bunch of whitespace errors which will need to be cleaned up but also the patch seems to fail to apply at all on the changes in test/clojure/test_clojure/numbers.clj. It looks like perhaps the diff is just not the right diff format. You might want to check out the instructions at http://dev.clojure.org/display/community/Developing+Patches about using git format-patch.

2) If you could put a more useful git commit message, that would be helpful. Something like "CLJ-1545 Adds missing unchecked-divide and unchecked-remainder for primitive longs."

Thanks!

Comment by Colin Taylor [ 04/Oct/14 4:47 PM ]

Uggh, sorry Alex.

New patch with better commit message.

Comment by Alex Miller [ 04/Oct/14 7:24 PM ]

The patch format looks better. Pulling out farther to the ticket itself, afaict Clojure will already use the right byteocode for checked or unchecked so this may not even be needed?

If I compile (without the patch):

(defn foo-div ^long [^long a ^long b]
  (quot a b))

then the bytecode for that fn is:

public final long invokePrim(long, long);
    Code:
       0: lload_1
       1: lload_3
       2: ldiv
       3: lreturn

similarly, quot of two longs yields the same code but with lrem. I think patch has no net effect on the resulting bytecode?

Comment by Andy Fingerhut [ 04/Oct/14 7:42 PM ]

Alex, did you do the testing in your previous comment with *unchecked-math* true or false? If false, then I would think that if CLJ-1254 is judged a bug, then the behavior you saw is a bug, too, that misses the same corner case.

Comment by Alex Miller [ 04/Oct/14 10:19 PM ]

The current results are the same with either unchecked-math setting, but I see your point.

Refreshing my memory of the (/ Long/MIN_VALUE -1) case, I think you're right. The (new) unchecked-divide / remainder should do what the current (checked) forms do and the regular division and remainder cases should be making the overflow check. I think CLJ-1254 should cover the quot changes.

Comment by Colin Taylor [ 04/Oct/14 10:19 PM ]

user=> (dotimes [_ 6] (time (dotimes [_ 50000000] (unchecked-divide 4 (System/currentTimeMillis)))))
"Elapsed time: 1806.942 msecs"
"Elapsed time: 1808.747 msecs"
"Elapsed time: 1865.074 msecs"
"Elapsed time: 1802.777 msecs"
"Elapsed time: 1839.468 msecs"
"Elapsed time: 1830.61 msecs"
nil
user=> (dotimes [_ 6] (time (dotimes [_ 50000000] (/ 4 (System/currentTimeMillis)))))
"Elapsed time: 5003.598 msecs"
"Elapsed time: 4998.182 msecs"
"Elapsed time: 4941.237 msecs"
"Elapsed time: 5036.517 msecs"
"Elapsed time: 4965.867 msecs"
"Elapsed time: 4982.693 msecs"





[CLJ-440] java method calls cannot omit varargs Created: 27/Sep/10  Updated: 04/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Alexander Taggart Assignee: Unassigned
Resolution: Unresolved Votes: 7
Labels: interop

Approval: Triaged

 Description   

From http://groups.google.com/group/clojure/browse_thread/thread/7d0d6cb32656a621

E.g., trying to call java.util.Collections.addAll(Collection c, T... elements)

user=> (Collections/addAll [] (object-array 0))
false
user=> (Collections/addAll [])
IllegalArgumentException No matching method: addAll  clojure.lang.Compiler$StaticMethodExpr.<init> (Compiler.java:1401)

The Method class provides an isVarArg() method, which could be used to inform the compiler to process things differently.



 Comments   
Comment by Assembla Importer [ 27/Sep/10 8:19 PM ]

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

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

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

Comment by Alexander Taggart [ 05/Apr/11 5:45 PM ]

Patch updated to current CLJ-445 patch.

Comment by Nick Klauer [ 29/Oct/12 8:12 AM ]

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.

Comment by Andy Fingerhut [ 29/Oct/12 10:45 AM ]

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

Comment by Andy Fingerhut [ 29/Oct/12 10:56 AM ]

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.





[CLJ-2] Scopes Created: 15/Jun/09  Updated: 27/Jul/13

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

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Rich Hickey
Resolution: Unresolved Votes: 2
Labels: None

Attachments: Text File scopes-spike.patch    

 Description   

Add the scope system for dealing with resource lifetime management



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

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

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

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

Comment by Stuart Halloway [ 12/Jul/11 8:26 AM ]

Patch demonstrates idea, not ready for prime time.

Comment by Tassilo Horn [ 23/Dec/11 7:37 AM ]

I think the decision of having to specify either a Closeable resource or a close function for an existing non-Closeable resource in with-open is quite awkward, because they have completely different meaning.

  (let [foo (open-my-custom-resource "foo.bar")]
    (with-open [r (reader "foo.txt")
                foo #(.terminate foo)]
      (do-stuff r foo)))

I think a CloseableResource protocol that can be extended to custom types as implemented in the patch to CLJ-308 is somewhat easier to use. Extend it once, and then you can use open-my-custom-resource in with-open just like reader/writer and friends...

That said, Scopes can still be useful, but I'd vote for handling the "how should that resource be closed" question by a protocol. Then the with-open helper can simply add

(swap! *scope* conj (fn [] (clojure.core.protocols/close ~(bindings 0))))

and cleanup-scope only needs to apply each fn without having to distinguish Closeables from fns.





[CLJ-1519] Added extra arity to clojure.core/ns-* fns Created: 04/Sep/14  Updated: 10/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Alex Baranosky Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: enhancement, patch

Attachments: Text File new-ns-arity.patch    
Patch: Code and Test

 Description   

Hello,

Adds another arity where the "ns" parameter is set to a default value of *ns* in these fns:

ns-unmap, ns-resolve, ns-name, ns-map, ns-publics, ns-imports, ns-interns, ns-refers, ns-aliases, ns-unalias

I find I very often use ns-unalias and ns-unmap from the repl, and passing the *ns* arg gets a little tedious.






[CLJ-899] Accept and ignore colon between key and value in map literals Created: 18/Dec/11  Updated: 19/Oct/14

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

Type: Enhancement Priority: Major
Reporter: Stuart Halloway Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: reader


 Description   

Original title was 'treat colons as whitespace' which isn't a problem description but a (flawed) implementation approach

For JSON compatibility
known problems when no spaces - x:true and y:false



 Comments   
Comment by Tassilo Horn [ 23/Dec/11 3:22 AM ]

Discussed here: https://groups.google.com/d/msg/clojure/XvJUzaY1jec/l8xEwlFl8EUJ

Comment by Kevin Downey [ 11/Jan/12 2:23 PM ]

please no

Comment by Tavis Rudd [ 16/Jan/12 12:17 PM ]

Alan Malloy raises a good point in the google group discussion (https://groups.google.com/d/msg/clojure/XvJUzaY1jec/aVpWBicwGhsJ) about accidental confusion between trailing (or floating) and leading colons:
"It isn't even as simple as "letting them
be whitespace", because presumably you want (read-string "{a: b}") to
result in (hash-map 'a 'b), but (read-string "{a :b}") to result in
(hash-map 'a :b)."

This issue could be avoided by only treating a colon as whitespace when followed by a comma. As easy cut-paste of json seems the be the key motivation here, the commas are going to be there anyway: valid {"v":, 1234} vs syntax error {a-key: should-be-a-keyword}.

Comment by Alex Baranosky [ 16/Jan/12 5:23 PM ]

This would be visually confusing imo.

Comment by Laurent Petit [ 17/Jan/12 5:01 PM ]

Please, oh please, no.

Comment by Tavis Rudd [ 18/Jan/12 2:40 PM ]

Er, brain fart. I was typing faster than I was thinking and put the comma in the wrong place. In my head I meant the form following the colon would have to have a comma after it. Thus, {"a-json-key": 1234, ...} would be valid while {"a-json-key": was-supposed-to-be-a-keyword "another-json-key" foo} would complain about the colon being an Invalid Token. I don't see the need for it, however.

Comment by Joseph Smith [ 27/Feb/12 10:55 AM ]

Clojure already has reader syntax for a map. If we support JSON, do we also support ruby map literals? Seems like this addition would only add confusion, imo, given colons are used in keywords and keywords are frequently used in maps - e.g., when de-serializing from XML, or even JSON.

Comment by David Nolen [ 27/Feb/12 11:19 AM ]

Clojure is no longer a language hosted only on the JVM. Clojure is also hosted on the CLR, and JavaScript. In particular ClojureScript can't currently easily deal with JSON literals - an extremely common (though problematic) data format. By allowing colon whitespace in map literals - Clojure data structures can effectively become an extensible JSON superset - giving the succinctness of JSON and the expressiveness of XML.

+1 from me.

Comment by Tim McCormack [ 13/Nov/12 7:27 PM ]

Clojure is only hosted on the JVM; ClojureScript is hosted on JS VMs. If this is useful for CLJS, it should just be a CLJS feature.

Comment by Mike Anderson [ 10/Dec/12 11:51 PM ]

-1 for this whole idea: that way madness lies....

If we keep adding syntactical oddities like this then the language will become unmaintainably complex. It's the exact opposite of simple to have lots of special cases and ambiguities that you have to remember.

If people want to use JSON that is fine, but then the best approach use a specific JSON parser/writer, not just paste it into Clojure source and expect it to work.

Comment by Laszlo Török [ 11/Dec/12 4:54 AM ]

-1 for reasons mentioned by Allan Malloy and Mike Anderson

Comment by Bozhidar Batsov [ 19/Oct/14 3:06 AM ]

-1 Don't repeat the mistake made in Ruby...





[CLJ-1530] Make foo/bar/baz unreadable Created: 22/Sep/14  Updated: 28/Oct/14

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

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

Attachments: Text File 0001-fix-LispReader-and-EdnReader-so-that-foo-bar-baz-is-.patch    
Patch: Code and Test

 Description   

Currently keywords and symbols containing more than one slash are disallowed by the spec, but allowed by the readers.
This trivial patch makes them unreadable by the readers too.

Pre:

user=> :foo/bar/baz
:foo/bar/baz

Post:

user=> :foo/bar/baz
RuntimeException Invalid token: :foo/bar/baz  clojure.lang.Util.runtimeException (Util.java:221)


 Comments   
Comment by Andy Fingerhut [ 22/Sep/14 12:14 PM ]

Perhaps overlap with CLJ-1527 ?

Comment by Thomas Engelschmidt [ 28/Oct/14 4:36 AM ]

Please notice that keywords with more than one slash has a different hashcode across clojure version 1.5 and 1.6

This creates a problem when using a datomic version that works with clojure 1.5 under clojure 1.6 and the schema have one or more keys with more than one slash.





[CLJ-1536] Remove usage of sun.misc.Signal (which may not be available in Java 9) Created: 26/Sep/14  Updated: 26/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

It looks like Java 9 will not continue to provide access to "internal" classes like sun.misc.Signal. Clojure currently uses this in the REPL to trap ctrl-c (SIGINT) and cancel current evaluation instead of process shutdown.

There is a page of alternatives here:
https://wiki.openjdk.java.net/display/JDK8/Java+Dependency+Analysis+Tool

But there is no suggested alternative for sun.misc.Signal and I'm not aware of a portable solution to it.






[CLJ-1582] Overriding in-ns and ns is problematic Created: 07/Nov/14  Updated: 07/Nov/14

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None

Attachments: Text File 0001-Allow-overriding-of-clojure.core-in-ns-and-clojure.c.patch    
Patch: Code

 Description   

Currently it is possible to override clojure.core/in-ns and clojure.core/ns, but it is not possible to refer to the namespace-specific vars without fully qualifying them:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
#<clojure.lang.RT$1@76b5e4c5>

After this patch, overriding in-ns and ns works like for every other clojure.core var:

user=> (ns foo (:refer-clojure :exclude [in-ns]))
nil
foo=> (def in-ns 1)
#'foo/in-ns
foo=> in-ns
1


 Comments   
Comment by Reid McKenzie [ 07/Nov/14 11:46 AM ]

This is motivated by https://github.com/jonase/eastwood/issues/100





[CLJ-1583] Apply forces the evaluation of one element more than necessary Created: 07/Nov/14  Updated: 09/Nov/14

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

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

Attachments: Text File 0001-make-RT.boundedLength-lazier.patch    
Patch: Code

 Description   

Given a function with one fixed argument and a vararg, it should be sufficient to force evaluation of 2 elements for apply to know which arity it should select, however it currently forces 3:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
2
nil

This makes lazy functions that use apply (for example mapcat) less lazy than they could be.
The proposed patch makes RT.boundedLength short-circuit immediately after the seq count is greater than the max fixed arity:

user=> (defn x ([a & b]))
#'user/x
user=> (apply x (map println (iterate inc 0)))
0
1
nil


 Comments   
Comment by Nicola Mometto [ 09/Nov/14 3:37 PM ]

The patch in this ticket slightly improves the issue reported at http://dev.clojure.org/jira/browse/CLJ-1218





[CLJ-1256] Support type-hinted overrides of function parameters Created: 06/Sep/13  Updated: 09/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: compiler, interop, typehints


 Description   

Problem: in many cases, the Clojure compiler has enough information about the type of a function argument to statically emit maximally efficient code on the JVM (i.e. without instance? checks, type casts or other forms of dynamic polymorphic dispatch). We are currently unable to do so in Clojure, which pushes developers with strong performance requirements to use some unidiomatic or convoluted workarounds.

Proposal is simply to allow functions to take type-hinted overloads of function arguments, e.g.

(defn foo
([^double x] (Math/floor x))
([^float x] (Math/floor (double x)))
([^String s] (count s)))

An "Object" version of the code with the correct arity will always be emitted, which will maintain compatibility with the IFn interface and ensure that the function can still be used in dynamic / interactive contexts. If the "Object" version is not explicitly provided, then it will be generated to use instance? checks that subsequently delegate to the appropriate typed version of the function (or throw an InvalidArgumentException if no match is found).

Matching rules would be the same as Java.

This will be backwards compatible with all existing uses of defn. In particular, it should extend / enhance / supercede the existing handling of primitive functions.

In the future, this technique might be used alongside core.typed to ensure that the most efficient function version is chosen based on type inference.






[CLJ-126] abstract superclass with non-public accessibility Created: 17/Jun/09  Updated: 05/Dec/14

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

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


 Description   

The following code works in Java 6 but not in Java 5:

(def Clojure 1.1.0-alpha-SNAPSHOT
user=> (def s (new StringBuilder "aaa"))
#'user/s
user=> (. s setCharAt (int 0) (char \a))
java.lang.Exception: Unable to resolve symbol: setCharAt in this context

This was discussed on the Clojure mailing list and Stephen C. Gillardi came up with the following conclusion:

_StringBuilder extends AbstractStringBuilder (though the JavaDoc docs lie and say it extends Object). AbstractStringBuilder has default accessibility (not public, protected, or private) which makes the class inaccessible to code outside the java.lang package. In both Java SE 5 and Java SE 6, StringBuilder does not contain a .setCharAt method definition. It relies on the inherited public method in AbstractStringBuilder. (I downloaded the source code for both versions from Sun to check.)

In Java SE 5, when Clojure checks whether or not .setCharAt on StringBuilder is public, it finds that it's a public method of a non-public base class and throws the exception you saw. (It looks like you're using a version of Clojure older than 18 May 2009 (Clojure svn r1371). Versions later than that print the more detailed message I saw.)

In Java SE 6, Clojure's checks for accessibility of this method succeed and the method call works.

I'm not sure whether or not Clojure could be modified to make this method call work in Java 5. Google searches turn up discussion that this pattern of using an undocumented abstract superclass with non-public accessibility is not common in the JDK._

This ticket is being filed in the event that Clojure can handle these types of situations somehow.



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

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

Comment by Assembla Importer [ 24/Aug/10 4:45 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 [ 24/Aug/10 4:45 AM ]

hiredman said: Related association with ticket #259 was added

Comment by Alex Miller [ 05/Dec/14 2:26 PM ]

possibly same as CLJ-1609





[CLJ-1612] clojure.core.reducers/mapcat can call f1 with undefined arity of 0 arguments? Created: 10/Dec/14  Updated: 10/Dec/14

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

Type: Defect Priority: Major
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers


 Description   

I have not run across this with running code, so perhaps it is impossible for reasons I have not understood. Also not sure whether fixing issues with reducers is of any importance, given transducers. This was found while testing the Eastwood lint tool on some Clojure namespaces, including clojure.core.reducers.

(defcurried mapcat
  "Applies f to every value in the reduction of coll, concatenating the result
  colls of (f val). Foldable."
  {:added "1.5"}
  [f coll]
  (folder coll
   (fn [f1]
     (let [f1 (fn
                ([ret v]
                  (let [x (f1 ret v)] (if (reduced? x) (reduced x) x)))
                ([ret k v]
                  (let [x (f1 ret k v)] (if (reduced? x) (reduced x) x))))]
       (rfn [f1 k]
            ([ret k v]
               (reduce f1 ret (f k v))))))))

The definition of macro rfn expands to a (fn ...) that can call f1 with no arguments, which is not a defined arity for f1.






[CLJ-1037] Allow doc strings for both interfaces and concrete implementations Created: 04/Aug/12  Updated: 17/Apr/14

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

Type: Enhancement Priority: Major
Reporter: Warren Lynn Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In this post
http://groups.google.com/group/clojure/browse_thread/thread/84de74740928da76#

I mentioned the rationale (I think) why this is important and needed. Thank you for consideration.



 Comments   
Comment by Kevin Downey [ 17/Apr/14 10:08 PM ]

clojure's documentation system has two parts:

1. docstrings are attached to the metadata of objects

2. the doc macro (and some other tools) read the docstrings from objects and display them

the two parts work together, without the doc macro, docstrings are just comments that also take up memory at runtime, and the doc macro has no purpose without the docstrings.

the two main places docstrings are hung are var metadata and namespace metadata.

for multimethods and protocol functions the docstrings are hung on vars.

for the implementations of multimethods and protocols there are no distinct vars to hang documentation information on, and it is not clear how you would look up those doc strings.

so to support docs on defmethods and protocol implementations would require enhancements to doc and some design work, so a wiki page to come up with a design would be a good idea.





[CLJ-1414] sort's docstring should say whether it is stable Created: 02/May/14  Updated: 28/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Phill Wolf Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: collections, docstring, ft

Attachments: Text File clj-1414-v1.patch    
Patch: Code
Approval: Triaged

 Description   

sort's docstring does not address whether the sort will be stable.

Stability is a useful property. It appears to be customary among programming tools to document whether their sort is stable. Java's Collections javadoc pledges a stable sort. The man-page of GNU coreutils sort in Ubuntu mentions its stability. The perldoc of Perl's sort function indicates it is a stable sort now but was not always.

Pillars of the Clojure community have commented on sort's stability:

(1) A recent book assembled by Cognitect consultants, "Clojure Cookbook", says Clojure's sort function "uses Java's built-in sort" and that "[t]he sort is also stable".

(2) In a 2011 discussion thread, "Clojure sort: is it specified to be stable for all targets?" https://groups.google.com/forum/#!topic/clojure/j3aNAmEJW9A , Stuart Sierra replied that "if it's not specified in the doc string, then it's not a promise. That said, [...] I would generally expect a language built-in `sort` routine to be stable, so take that for what it's worth."

Screened by: Alex Miller



 Comments   
Comment by Alex Miller [ 05/May/14 10:23 AM ]

Sounds reasonable. Needs patch from contributor.

Comment by Andy Fingerhut [ 30/Aug/14 3:07 PM ]

Patch clj-1414-v1.patch dated Aug 30 2014 adds the sentence "Guaranteed to be stable: equal elements will not be reordered." to the doc strings of both sort and sort-by.





[CLJ-1488] Implement Named over Vars Created: 01/Aug/14  Updated: 28/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Reid McKenzie Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-Implement-clojure.lang.Named-over-Vars.patch    
Patch: Code
Approval: Triaged

 Description   

Vars, while a general reference structure, are used to implement bindings and have special reader and printer notation reflecting this reality. Unlike Keywords and Symbols which share the "namespace/name" notation of Vars, Vars do not implement the clojure.lang.Named interface while they print as if they were Named.

The attached patch implements Named over Vars.

Example:

user=> (name :clojure.core/conj)
"conj"
user=> (namespace :clojure.core/conj)
"clojure.core"
user=> (name 'clojure.core/conj)
"conj"
user=> (namespace 'clojure.core/conj)
"clojure.core"
user=> (name #'clojure.core/conj)
"conj"
user=> (namespace #'clojure.core/conj)
"clojure.core"
user=> (with-local-vars [x 1] (name x))
"--unnamed--"
user=> (with-local-vars [x 1] (namespace x))
nil
user=> (with-local-vars [x 1] (println x))
#<Var: --unnamed-->

This is useful for applications such as the CinC project where Vars are often taken directly as values in which context they would ideally be interchangeable with the Symbols the bound values of which they represent.



 Comments   
Comment by Nicola Mometto [ 02/Aug/14 11:42 AM ]

With this patch calling `name` on a unnamed Var will cause a NPE, I don't think this is desiderable.

Comment by Reid McKenzie [ 02/Aug/14 1:39 PM ]

I agree, however this behavior seems to be standard in Core.

Clojure 1.6.0
user=> (name nil)
NullPointerException clojure.core/name (core.clj:1518)
user=> (namespace nil)
NullPointerException clojure.core/namespace (core.clj:1526)

I'm also not convinced that the "name" or "namespace" of an unbound var is meaningful, in which case a NPE is probably acceptable.

Comment by Nicola Mometto [ 02/Aug/14 1:45 PM ]

I was not talking about unbound Vars, but about anonymous Vars, I'm assuming you miswrote.

I'd agree with you that throwing an exception could be a reasonable behaviour, except I can test for nil before calling name on it while there's no way to test whether a var is named or not, except trying to access directly the .name field which is excatly what this ticket is for.

Comment by Nicola Mometto [ 02/Aug/14 2:27 PM ]

Me and Reid have been talking about this issue over IRC, here's what's come up:

  • Vars can be either unnamed (as are Vars returned by with-local-vars) or contain both a namespace and a name part( that's the case for interned Vars)
  • there's currently no way to test for the "internedness" of a Var, so accessing either the .name or the .namespace field of the Var testing for nil is the only way to do it currently

given the above, the current patch seems unsatisfactory, here some proposed solutions:

  • make Var Named, make namespace return nil for an unnamed Var and name return "--unnamed--"
  • keep Var not implementing Named, add a "var-symbol" function returning either a namespaced symbol matching the ns+name of the Var or nil for an unnamed Var

Personally, I'd rather have the second solution implemented as I don't feel Var should be Named given that they can be unnamed and that strikes me as a contradicion

Comment by Reid McKenzie [ 02/Aug/14 3:16 PM ]

Added patches explicitly handling the unnamed var cases.

Comment by Reid McKenzie [ 02/Aug/14 3:33 PM ]

Squashed all patches into a single diff and updated attachments.





[CLJ-1629] Improve error message when defn form omits parameter declaration Created: 29/Dec/14  Updated: 29/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Sanel Zukan Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs
Environment:

Reproducible on all platforms and all clojure versions.


Approval: Triaged

 Description   

When defn form is malformed, Clojure compiler will report meaningless error and in combination with function body, can cause really bad experience. Here is the sample:

(defn foo
  "This is docstring."
  (let [i 1]
    (+ i 1)))

It will report:

IllegalArgumentException Parameter declaration "let" should be a vector  clojure.core/assert-valid-fdecl (core.clj:7123)

However, if is written:

(defn foo "bla")

error report makes more sense:

IllegalArgumentException Parameter declaration missing  clojure.core/assert-valid-fdecl (core.clj:7107)


 Comments   
Comment by Michael Blume [ 29/Dec/14 1:39 PM ]

I don't think this is really meaningless – if you replace the symbol let with a vector, say, [i], you get a perfectly valid function definition

(defn foo
  "This is docstring."
  ([i] [i 1]
    (+ i 1)))
Comment by Sanel Zukan [ 29/Dec/14 2:41 PM ]

Yes and maybe make sense for this case. But in general, the report is misleading for common defn forms (how often you will see function definitions written this way, unless you want multi-arity function) and should have the same report as for second sample; in both cases it is the same cause.





[CLJ-1630] Destructuring allows multiple &-params Created: 31/Dec/14  Updated: 01/Jan/15

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

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

Attachments: Text File no-multiple-rest-params-v1.patch    
Patch: Code and Test

 Description   

(let [[foo & bar & baz] []]) compiles and probably shouldn't.



 Comments   
Comment by Alex Miller [ 01/Jan/15 10:17 AM ]

I see:

user=> (defn foo [bar & baz & qux])

CompilerException java.lang.RuntimeException: Invalid parameter list, compiling:(/private/var/folders/7r/_1fj0f517rgcxwx79mn79mfc0000gn/T/form-init3743582784321941885.clj:1:1)

?

Comment by Michael Blume [ 01/Jan/15 12:27 PM ]

Sorry, I was working on memory rather than actually typing the thing I put in the description into a REPL, which was dumb.

user=> (let [[foo & bar & baz] []])
nil





[CLJ-1643] Generative test for sequence implementations Created: 15/Jan/15  Updated: 15/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Michael Blume Assignee: Michael Blume
Resolution: Unresolved Votes: 0
Labels: generative-test, test

Attachments: Text File clj-1643-gen-seq-test-v1.patch    
Patch: Code and Test
Approval: Triaged

 Description   

This is an attempt to write a minimal-foreknowledge failing test for CLJ-1633. By minimal-foreknowledge, I mean a test that fails in the presence of the bug, but which one could imagine writing without intimate knowledge of the details of the bug. I suspect that looking for tests like this is a good way to find gaps in test coverage, and produce tests that will uncover novel regressions later on.

Approach: Generate a single list of operations that could be performed on a sequence, changing that sequence. Make two copies of that operation list, and insert what should be identity-preserving operations into each. Run the two lists of operations and verify that the final results are the same.

With CLJ-1633 unfixed, we get this output:

[java] Testing clojure.test-clojure.sequences
     [java]
     [java] FAIL in (seq-gentest) (sequences.clj:135)
     [java] {:acts1 (->> nil (cons :foo) (cons :foo) into-array next (apply list)),
     [java]  :acts2 (->> nil (cons :foo) (cons :foo) next),
     [java]  :result1 (:foo :foo),
     [java]  :result2 (:foo),
     [java]  :pass false}
     [java]
     [java] expected: (:result res)
     [java]   actual: false





[CLJ-1534] Adding condp-> and condp->> macros to core library Created: 24/Sep/14  Updated: 28/Jan/15

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

Type: Enhancement Priority: Major
Reporter: Kuldeep Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: enhancement, macro

Attachments: File clj_1534.diff     File condp-threading-macros-25sept2014.diff    
Patch: Code

 Description   

After introduction of cond-> and cond->> macros in 1.5. It makes sense to have condp-> and condp->> macros in the core library.

(condp-> {}
(complement :a) (assoc :a 1)
:a (assoc :b 2)) ;=> {:b 2, :a 1}

In the above example the result of each expr which was evaluated is being passed to the next predicate.



 Comments   
Comment by Andy Fingerhut [ 01/Oct/14 6:37 PM ]

Kuldeep, I cannot comment on whether this change is of interest to the Clojure developers, because I do not know.

I can say that the patch you have attached is not in the expected format. See the page below for instructions on creating a patch in the expected format:

http://dev.clojure.org/display/community/Developing+Patches

Comment by Kuldeep [ 28/Jan/15 11:31 AM ]

Rebased against master and generated patch as described in wiki.





[CLJ-1376] Initialize internal maps to more efficient version Created: 11/Mar/14  Updated: 11/Mar/14

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

Type: Enhancement Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: performance


 Description   

In reviewing some hashing stuff, I noticed that there are many places internal to Clojure that use maps initialized with PersistentHashMap.EMPTY. Many of these maps are likely to have a small number of entries such that a PersistentArrayMap might be more efficient.

These are the candidates:

src/jvm/clojure/lang/ARef.java
19:private volatile IPersistentMap watches = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Compiler.java
3009:				IPersistentMap m = PersistentHashMap.EMPTY;
3819:					       KEYWORDS, PersistentHashMap.EMPTY,
3820:					       VARS, PersistentHashMap.EMPTY,
3964:	IPersistentMap closes = PersistentHashMap.EMPTY;
3977:	IPersistentMap keywords = PersistentHashMap.EMPTY;
3978:	IPersistentMap vars = PersistentHashMap.EMPTY;
5121:                            ,CLEAR_SITES, PersistentHashMap.EMPTY
7259:			       KEYWORDS, PersistentHashMap.EMPTY,
7260:			       VARS, PersistentHashMap.EMPTY
7418:			IPersistentMap opts = PersistentHashMap.EMPTY;
7475:			IPersistentMap fmap = PersistentHashMap.EMPTY;
7522:					       KEYWORDS, PersistentHashMap.EMPTY,
7523:					       VARS, PersistentHashMap.EMPTY,
7912:                            ,CLEAR_SITES, PersistentHashMap.EMPTY

src/jvm/clojure/lang/LispReader.java
755:					RT.map(GENSYM_ENV, PersistentHashMap.EMPTY));

src/jvm/clojure/lang/MultiFn.java
39:	this.methodTable = PersistentHashMap.EMPTY;
41:	this.preferTable = PersistentHashMap.EMPTY;
49:		methodTable = methodCache = preferTable = PersistentHashMap.EMPTY;

src/jvm/clojure/lang/Var.java
48:	final static Frame TOP = new Frame(PersistentHashMap.EMPTY, null);
175:	setMeta(PersistentHashMap.EMPTY);
341:	IPersistentMap ret = PersistentHashMap.EMPTY;

Approach: Two possible approaches - initialize to PersistentArrayMap.EMPTY or call RT.map(). The latter requires function invocation so is slightly slower, but has the benefit of localizing map construction into a single place.






[CLJ-1279] Fix confusing macroexpand1 ArityException handling Created: 16/Oct/13  Updated: 05/Feb/14

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

Type: Defect Priority: Major
Reporter: Alex Coventry Assignee: Unassigned
Resolution: Unresolved Votes: 3
Labels: Compiler, errormsgs, macro

Attachments: Text File 0001-Edit-macro-ArityException-in-AFn.patch     Text File 0001-Fix-macroexpand1-s-handling-of-ArityException.patch    
Patch: Code and Test

 Description   

macros can give very confusing error messages when they execute a form which generates an ArityException. clojure.lang.Compiler.macroexpand1 assumes that any ArityException comes from the call to the macro itself, which need not be the case. For instance:

user> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (-2) passed to: core$assoc clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
user> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (-2) passed to: core$assoc
clojure.lang.Compiler.macroexpand1 (Compiler.java:6488)
clojure.lang.Compiler.macroexpand (Compiler.java:6544)
clojure.lang.Compiler.eval (Compiler.java:6618)
clojure.lang.Compiler.eval (Compiler.java:6624)
clojure.lang.Compiler.eval (Compiler.java:6597)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6596/fn-6599 (main.clj:260)
clojure.main/repl/read-eval-print--6596 (main.clj:260)
clojure.main/repl/fn--6605 (main.clj:278)
clojure.main/repl (main.clj:278)
clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn--1251 (interruptible_eval.clj:56)
clojure.core/apply (core.clj:617)
nil

Easy enough to see the source of the problem in this case, but because both the number of arguments actually passed is off by two, and the stacktrace element for the call to assoc has been dropped, this shortcut by macroexpand1 can get super confusing.

The attached patch corrects this behavior. E.g.

user=> (do (defmacro f [] (assoc)) (f))
ArityException Wrong number of args (0) passed to: core$assoc clojure.lang.AFn.throwArity (AFn.java:437)
user=> (use 'clojure.repl) (pst)
nil
ArityException Wrong number of args (0) passed to: core$assoc
user/f (NO_SOURCE_FILE:1)
clojure.lang.Var.invoke (Var.java:419)
clojure.lang.Var.applyTo (Var.java:532)
clojure.lang.Compiler.macroexpand1 (Compiler.java:6507)
clojure.lang.Compiler.macroexpand (Compiler.java:6580)
clojure.lang.Compiler.eval (Compiler.java:6654)
clojure.lang.Compiler.eval (Compiler.java:6660)
clojure.lang.Compiler.eval (Compiler.java:6633)
clojure.core/eval (core.clj:2864)
clojure.main/repl/read-eval-print-6594/fn-6597 (main.clj:260)
clojure.main/repl/read-eval-print--6594 (main.clj:260)
clojure.main/repl/fn--6603 (main.clj:278)
nil



 Comments   
Comment by Alex Coventry [ 17/Oct/13 11:01 AM ]

Patch with test

Comment by Alex Coventry [ 23/Oct/13 11:42 PM ]

Amended patch to deal more gracefully with unexpected stack trace structure.

Comment by Alex Miller [ 24/Oct/13 12:09 AM ]

Also see CLJ-397 and CLJ-383.

Comment by Alex Coventry [ 24/Oct/13 2:46 PM ]

Thanks, Alex. It would be easy enough to move most of the logic into ArityException, which would be a compromise between Stu's[1] options 1 and 2. Is that worth doing?

Amending clojure.lang.AFn.throwArity to check whether "this" is a macro and adjust the arg count there accordingly might be the simplest way. I can see why Rich prefers all the logic to go into ArityException, but since ArityExceptions are used for things other than macros, I don't see a way to make an honest error message there without groveling the stack trace.

[1] http://dev.clojure.org/jira/browse/CLJ-397?focusedCommentId=24090&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-24090

Comment by Alex Miller [ 24/Oct/13 2:57 PM ]

I would have to take more time than I have to make an informed opinion but I can say that from a general point of view inspecting StackTraceElements does not seem like the right solution to (almost) any problem.

Comment by Alex Coventry [ 24/Oct/13 10:26 PM ]

This patch causes Var.setMacro to set instance attribute AFn.macrop to true, so that AFn.throwArity can reduce the number of arguments reported.

I'm not used to negotiating java class hierarchies, so it's possible there's a cleaner way. Since Var.fn() returns an IFn, I added macrop handling methods IFn.setMacro and IFn.isMacro. These then needed to be implemented in Ref and Keyword, as well as AFn (where I wanted them) because they implement the IFn interface but don't inherit from AFn.

The real drawback I see with this approach is the duplicated state, though: ^{:macro true} vs AFn.macrop==true.

Comment by Andy Fingerhut [ 25/Oct/13 6:33 PM ]

I have not investigated the reason yet, but neither patch applies cleanly after the latest commits to Clojure master on Oct 25 2013. Given that what kinds of solution methods would be acceptable for this issue, it sounds like more thinking and code changes are probably needed anyway before worrying too much about that.





[CLJ-864] defrecord positional arity factory fn should have an inline version that calls the record constructor Created: 26/Oct/11  Updated: 04/Dec/13

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

Type: Enhancement Priority: Major
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord


 Description   

defrecord positional arity factory fn should have an inline version that calls the record constructor



 Comments   
Comment by Gary Fredericks [ 26/May/13 3:39 PM ]

I had the idea recently that the factory fn was useful partly for being a redefinable var, e.g. that you could wrap with contracts or anything else. This idea would preclude that.

It makes sense though if the only purpose of ->Foo is to avoid having to :import something.

Comment by Kevin Downey [ 13/Aug/13 4:04 PM ]

interesting, that is a good point

Comment by Gary Fredericks [ 04/Dec/13 7:21 AM ]

Another thought – using the factory fns rather than constructors directly gives you a little bit of protection against code-reloading issues, does it not? I don't think I understand the code reloading issues in great detail, so I'm not confident about this. My assumption is that the compiled code refers to vars rather than classes.





[CLJ-792] Refactor method resolution code out of Compiler and into Reflector Created: 11/May/11  Updated: 30/Jan/15

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

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

Attachments: Text File clj-792-reorg-reflector-patch2.txt     Text File clj-792-v3.patch     Text File clj-793-v4.patch     Text File clj-793-v5.patch     Text File reorg-reflector.patch    

 Description   

Issues:

  1. Code for obtaining method/constructor instances is duplicated across the Compiler
  2. Code for resolving a preferred overloaded method lives in the Compiler

By consolidating the duplicated code, moving the reflection-related parts into Reflector, and providing a straightforward API, it should be easier to read and understand the method resolution process. Further, improvements to (e.g., CLJ-445) the mechanism for reflecting on class members can largely be isolated from the Compiler. And the few points of coordination (e.g., Compiler emitting same arg and return types as Reflector does when invoking) can be clearly identified and documented.



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

Patch does not apply as of commit f5bcf64.

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

Yeah, year-old patches tend to do that.

Comment by Andy Fingerhut [ 20/Feb/12 1:11 PM ]

I don't know if this is helpful or not, but this updated version of Alexander's patch applies cleanly to latest Clojure head as of Feb 20, 2012. It compiles, but does not pass ant test.

Comment by Nicola Mometto [ 30/Jan/15 7:29 AM ]

I would be very interested in having this patch considered again and if there's interest I could help with porting the patch to the current master.

I believe anybody who spent some time between Compiler.java and Reflector.java would agree that this refactoring/cleanup has been long needed.

Comment by Michael Blume [ 30/Jan/15 12:22 PM ]

I've attempted to forward-port the patch again. The Java code compiles, and then is able to compile the Clojure source, but then fails in compiling the clojure tests with

test-example:
     [java] Exception in thread "main" java.lang.IllegalArgumentException: Found multiple uncheckedByteCast methods in clojure.lang.RT for argtypes: byte, compiling:(clojure/test_clojure/numbers.clj:127:3)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6568)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6556)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:981)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6556)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5693)
     [java] 	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:6007)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.access$200(Compiler.java:38)
     [java] 	at clojure.lang.Compiler$LetExpr$Parser.parse(Compiler.java:5957)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5693)
     [java] 	at clojure.lang.Compiler$TryExpr$Parser.parse(Compiler.java:2182)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5693)
     [java] 	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5124)
     [java] 	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3753)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6559)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:3565)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6563)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$TryExpr$Parser.parse(Compiler.java:2153)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5691)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5691)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$BodyExpr$Parser.parse(Compiler.java:5693)
     [java] 	at clojure.lang.Compiler$FnMethod.parse(Compiler.java:5124)
     [java] 	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3753)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6559)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6549)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$MapExpr.parse(Compiler.java:2882)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6360)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:559)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6352)
     [java] 	at clojure.lang.Compiler.analyze(Compiler.java:6313)
     [java] 	at clojure.lang.Compiler.eval(Compiler.java:6624)
     [java] 	at clojure.lang.Compiler.load(Compiler.java:7047)
     [java] 	at clojure.lang.RT.loadResourceScript(RT.java:370)
     [java] 	at clojure.lang.RT.loadResourceScript(RT.java:361)
     [java] 	at clojure.lang.RT.load(RT.java:440)
     [java] 	at clojure.lang.RT.load(RT.java:411)
     [java] 	at clojure.core$load$fn__5424.invoke(core.clj:5848)
     [java] 	at clojure.core$load.doInvoke(core.clj:5847)
     [java] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
     [java] 	at clojure.core$load_one.invoke(core.clj:5653)
     [java] 	at clojure.core$load_lib$fn__5373.invoke(core.clj:5693)
     [java] 	at clojure.core$load_lib.doInvoke(core.clj:5692)
     [java] 	at clojure.lang.RestFn.applyTo(RestFn.java:142)
     [java] 	at clojure.core$apply.invoke(core.clj:628)
     [java] 	at clojure.core$load_libs.doInvoke(core.clj:5731)
     [java] 	at clojure.lang.RestFn.applyTo(RestFn.java:137)
     [java] 	at clojure.core$apply.invoke(core.clj:628)
     [java] 	at clojure.core$require.doInvoke(core.clj:5814)
     [java] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
     [java] 	at user$eval72.invoke(run_test.clj:6)
     [java] 	at clojure.lang.Compiler.eval(Compiler.java:6620)
     [java] 	at clojure.lang.Compiler.load(Compiler.java:7047)
     [java] 	at clojure.lang.Compiler.loadFile(Compiler.java:7003)
     [java] 	at clojure.main$load_script.invoke(main.clj:274)
     [java] 	at clojure.main$script_opt.invoke(main.clj:336)
     [java] 	at clojure.main$main.doInvoke(main.clj:420)
     [java] 	at clojure.lang.RestFn.invoke(RestFn.java:408)
     [java] 	at clojure.lang.Var.invoke(Var.java:379)
     [java] 	at clojure.lang.AFn.applyToHelper(AFn.java:154)
     [java] 	at clojure.lang.Var.applyTo(Var.java:700)
     [java] 	at clojure.main.main(main.java:37)
     [java] Caused by: java.lang.IllegalArgumentException: Found multiple uncheckedByteCast methods in clojure.lang.RT for argtypes: byte
     [java] 	at clojure.lang.Reflector.getMatchingParams(Reflector.java:385)
     [java] 	at clojure.lang.Reflector.getMatchingMember(Reflector.java:419)
     [java] 	at clojure.lang.Reflector.getMatchingMethod(Reflector.java:485)
     [java] 	at clojure.lang.Reflector.getMatchingStaticMethod(Reflector.java:499)
     [java] 	at clojure.lang.Compiler$StaticMethodExpr.<init>(Compiler.java:1578)
     [java] 	at clojure.lang.Compiler$HostExpr$Parser.parse(Compiler.java:983)
     [java] 	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6561)
     [java] 	... 110 more
Comment by Michael Blume [ 30/Jan/15 12:55 PM ]

clj-793-v4 has the useful property of compiling on every commit, for easier bisection, though the tests still don't run

The tests seem to start failing at "move getMatchingParams method from Compiler to Reflector" which looks like an entirely innocuous change to me, so I'm a bit confused.

Comment by Michael Blume [ 30/Jan/15 1:08 PM ]

Aha, tried just moving getMatchingParams using IntelliJ refactor and comparing, looks like the assign "tied = false" somehow got dropped. That fixes that test error and leaves a new one. Bisecting...

Comment by Michael Blume [ 30/Jan/15 1:12 PM ]

By the way I'm very grateful to Alexander Taggart for authoring this patch as a series of small commits so that it could be relatively easily forward-ported/bisected.

Comment by Michael Blume [ 30/Jan/15 1:29 PM ]

ok, this has the tied=false fix, but there's a new problem

In "Moved instance-method-finding code from Compiler to Reflector" the code that's being moved out of Compiler has two different calls to RT.errPrintWriter which use lots of information that's only available in the Compiler (column, row, etc). When this code got moved to the Reflector, I tried to be clever in my forward-port and just leave the error-printing in one spot in the Compiler, but that doesn't work because the two calls are printing different messages. I'm not actually sure what a good way to move this to Reflector would be.

Relevant change is also at https://github.com/MichaelBlume/clojure/commit/56995f3376795a3cfcfc6339e7d8ad24d8459d31

look for the calls to RT.errPrintWriter and you'll see what changed.

Comment by Michael Blume [ 30/Jan/15 1:44 PM ]

To summarize perhaps a bit better, there's some code that this patch moves from Compiler into its own method in Reflector that was relatively easy to move back when this patch was first authored, but now has some extra reflection-warning-printing that wants to know row/column stuff that's available in its original location in Compiler but not in the extracted function in Reflector. We could pass row/column into that method, but then we'd have to eventually pass them through a few other Reflector methods and that sounds ugly. Alternately we could pass some indication out of that reflector method of how exactly it failed to find a method so that the Compiler method could make its own decisions about how to print the failure, but that would involve either inventing exceptions or some sort of Success/Failure ADT.

Comment by Nicola Mometto [ 30/Jan/15 1:45 PM ]

Michael, what about doing

if (Reflector.getMethods(target.getJavaClass(), args.count(), methodName, false) == null)
   RT.errPrintWriter().format("Reflection warning, %s:%d:%d - call to method %s on %s can't be resolved (no such method).\n", SOURCE_PATH.deref(), line, column, methodName, target.getJavaClass().getName());
else
   RT.errPrintWriter().format("Reflection warning, %s:%d:%d - call to method %s on %s can't be resolved (argument types: %s).\n",SOURCE_PATH.deref(), line, column, methodName, target.getJavaClass().getName(), getTypeStringForArgs(args));

inside the block starting with

if(method == null && RT.booleanCast(RT.WARN_ON_REFLECTION.deref()))

in Compiler.java?

Comment by Michael Blume [ 30/Jan/15 2:08 PM ]

That makes sense, it runs into trouble later when Reflector.getMethods gets replaced with the private getMethodsForName but maybe we can hack around that...

Comment by Michael Blume [ 30/Jan/15 2:27 PM ]

No, still doesn't print out the right warnings

[java] Testing clojure.test-clojure.rt
     [java]
     [java] FAIL in (error-messages) (rt.clj:43)
     [java] reflection cannot resolve instance method because it is missing
     [java] expected: (clojure.core/re-matches #"Reflection warning, .*:\d+:\d+ - call to method zap on java\.lang\.String can't be resolved \(no such method\)\.\r?\n" (clojure.test-helper/with-err-string-writer (clojure.test-helper/eval-in-temp-ns (defn foo [x] (.zap x 1)))))
     [java]   actual: (not (clojure.core/re-matches #"Reflection warning, .*:\d+:\d+ - call to method zap on java\.lang\.String can't be resolved \(no such method\)\.\r?\n" "Reflection warning, /Users/michael.blume/workspace/clojure/src/script/run_test.clj:45:28 - call to method zap on java.lang.String can't be resolved (argument types: long).\n"))
     [java]
     [java] FAIL in (error-messages) (rt.clj:43)
     [java] reflection cannot resolve instance method because it is missing
     [java] expected: (clojure.core/re-matches #"Reflection warning, .*:\d+:\d+ - call to method zap on java\.lang\.String can't be resolved \(no such method\)\.\r?\n" (clojure.test-helper/with-err-print-writer (clojure.test-helper/eval-in-temp-ns (defn foo [x] (.zap x 1)))))
     [java]   actual: (not (clojure.core/re-matches #"Reflection warning, .*:\d+:\d+ - call to method zap on java\.lang\.String can't be resolved \(no such method\)\.\r?\n" "Reflection warning, /Users/michael.blume/workspace/clojure/src/script/run_test.clj:45:28 - call to method zap on java.lang.String can't be resolved (argument types: long).\n"))
Comment by Michael Blume [ 30/Jan/15 2:29 PM ]

Patch series in v5 is also at https://github.com/MichaelBlume/clojure/commits/compile-refactor
Patch series with Nicolo's suggestion is at https://github.com/MichaelBlume/clojure/commits/compile-refactor-nicolo





[CLJ-1662] folding over hash-map nested hash-map throws exception Created: 17/Feb/15  Updated: 17/Feb/15

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

Type: Defect Priority: Major
Reporter: Francis Avila Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers
Environment:

JVM 1.7.0_76



 Description   

I got a baffling exception in a recursive function that folds. REPL transcript below:

nREPL server started on port 57818 on host 127.0.0.1 - nrepl://127.0.0.1:57818
REPL-y 0.3.5, nREPL 0.2.6
Clojure 1.7.0-alpha5
Java HotSpot(TM) 64-Bit Server VM 1.7.0_76-b13
    Docs: (doc function-name-here)
          (find-doc "part-of-name-here")
  Source: (source function-name-here)
 Javadoc: (javadoc java-object-or-class-here)
    Exit: Control+D or (exit) or (quit)
 Results: Stored in vars *1, *2, *3, an exception in *e

user=> (use 'foldtest.core)
nil
user=> (source leafs)
(defn leafs [xs]
  (->> (r/mapcat (fn [k v]
                   (if (map? v)
                     (leafs v)
                     [[k v]])) xs)
       (r/foldcat)))
nil
user=> (leafs (hash-map :a (hash-map :b 1 :c 2)))

ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn  clojure.core.reducers/fjinvoke (reducers.clj:48)
user=> (pst)
ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn
	clojure.core.reducers/fjinvoke (reducers.clj:48)
	clojure.lang.PersistentHashMap.fold (PersistentHashMap.java:207)
	clojure.core.reducers/eval1347/fn--1348 (reducers.clj:367)
	clojure.core.reducers/eval1220/fn--1221/G--1211--1232 (reducers.clj:81)
	clojure.core.reducers/folder/reify--1247 (reducers.clj:130)
	clojure.core.reducers/fold (reducers.clj:98)
	clojure.core.reducers/fold (reducers.clj:96)
	clojure.core.reducers/foldcat (reducers.clj:318)
	foldtest.core/leafs (core.clj:5)
	foldtest.core/leafs/fn--1367 (core.clj:7)
	clojure.core.reducers/mapcat/fn--1277/fn--1280 (reducers.clj:185)
	clojure.lang.PersistentHashMap$NodeSeq.kvreduce (PersistentHashMap.java:1127)
nil
user=>

Note that it must be a hash-map nested in a hash-map. Other combinations of array and hash maps seem fine:

user=> (leafs (array-map :a (hash-map :b 1 :c 2)))
[[:c 2] [:b 1]]
user=> (leafs (hash-map :a (array-map :b 1 :c 2)))
[[:b 1] [:c 2]]
user=> (leafs (hash-map :a (hash-map :b 1 :c 2)))

ClassCastException clojure.lang.PersistentHashMap$1 cannot be cast to clojure.lang.IFn  clojure.core.reducers/fjinvoke (reducers.clj:48)
user=> (leafs (array-map :a (array-map :b 1 :c 2)))
[[:b 1] [:c 2]]
user=>

Possibly related: CLJCLR-63

It took me a while to discover this because of this inconsistency (which I am not sure is a bug):

user=> (def a {:a 1})
#'user/a
user=> (type a)
clojure.lang.PersistentHashMap
user=> (let [a {:a 1}] (type a))
clojure.lang.PersistentArrayMap
user=> (type {:a 1})
clojure.lang.PersistentArrayMap
user=>

(I had put test input in a def, but using the defed var always failed but literals always worked!)






[CLJ-1665] take-nth transducer could be faster without rem Created: 20/Feb/15  Updated: 20/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Steve Miner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: performance
Environment:

Mac OS X 10.10.2, JDK 1.8.0_31


Attachments: Text File CLJ-1665-faster-take-nth-transducer-without-rem.patch    
Patch: Code
Approval: Triaged

 Description   

The take-nth transducer is calling rem on each index, which is relatively expensive compared to a zero? test. It could just count down from N instead as the step size is fixed.



 Comments   
Comment by Steve Miner [ 20/Feb/15 12:34 PM ]

Patch attached. It's about 25% faster on a simple test like:

(time (transduce (take-nth 13) + (range 1e7)))
Comment by Steve Miner [ 20/Feb/15 12:41 PM ]

I didn't worry about (take-nth 0) case, but my patch does give a different result. The current implementation gets a divide by zero error (from rem). My patched version returns just the first element once. The regular collection version returns an infinite sequence of the first element. I doubt anyone expects a sensible answer from the 0 case so I didn't try to do anything special with it.

Comment by Michael Blume [ 20/Feb/15 12:55 PM ]

Nice =)

I would say that the transducer version really ought to match the collection version as closely as possible, but I don't think there's actually a way to write a transducer that transforms a finite sequence into an infinite sequence, so no luck there.

Maybe while we're at it we should change both the transducer and the collection arities to throw on zero?





[CLJ-77] GC Issue 74: Clojure compiler emits too-large classfiles (results in ClassFormatError) Created: 17/Jun/09  Updated: 23/Feb/15

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

Type: Defect Priority: Major
Reporter: Chas Emerick Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   
Reported by cemer...@snowtide.com, Feb 10, 2009

The jvm has certain implementation limits around the maximum size of
classfiles, literal strings, method length, etc; however, in certain
circumstances, the Clojure compiler can currently emit classfiles that
violate some of those limitations, causing an error later when the
classfile is loaded.

While test coverage would necessarily detect this sort of problem on a
project-by-project basis when one's tests attempted to load a project's
classfiles, it seems like Clojure should do the following to ensure failure
as quickly as possible:

- throw an exception immediately if, while compiling a lib, it is detected
that the resulting classfile(s) would violate any classfile implementation
limits.  Ideally, the exception's message would detail what file and on
which line number the offending form is (e.g. if a method's bytecode would
be too long).  I can imagine that doing this may not be straightforward; a
reasonable stop-gap would be for the compiler to immediately attempt to
load the generated classfile in order to ensure up-front failure.

- emit a warning if any clojure form is read that would, upon being
compiled, require violating any of the classfile implementation limits; I
suspect that *most* people looking to generate classfiles would be doing so
in a "build" environment (rather than loading some code, tinkering, and
then using clojure.core/compile), but for those that aren't, I can imagine
there being a good deal of frustration around seeing that loading and using
some code successfully would eventually produce unusable classfiles.

I've appended a sample stack trace emitted by java when it attempted to
load a too-long method implementation (which was produced by embedding a
large list literal in a compiled lib).

Exception in thread "main" java.lang.ClassFormatError: Invalid method  
Code length 105496 in class file com/foo/MyClass__init
         at java.lang.ClassLoader.defineClass1(Native Method)
         at java.lang.ClassLoader.defineClass(ClassLoader.java:675)
         at  
java.security.SecureClassLoader.defineClass(SecureClassLoader.java:124)
         at java.net.URLClassLoader.defineClass(URLClassLoader.java:260)
         at java.net.URLClassLoader.access$000(URLClassLoader.java:56)
         at java.net.URLClassLoader$1.run(URLClassLoader.java:195)
         at java.security.AccessController.doPrivileged(Native Method)
         at java.net.URLClassLoader.findClass(URLClassLoader.java:188)
         at java.lang.ClassLoader.loadClass(ClassLoader.java:316)
         at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:
288)
         at java.lang.ClassLoader.loadClass(ClassLoader.java:251)
         at java.lang.ClassLoader.loadClassInternal(ClassLoader.java:
374)
         at java.lang.Class.forName0(Native Method)
         at java.lang.Class.forName(Class.java:247)
         at clojure.lang.RT.loadClassForName(RT.java:1512)
         at clojure.lang.RT.load(RT.java:394)
         at clojure.lang.RT.load(RT.java:374)
         at clojure.core$load__4911$fn__4913.invoke(core.clj:3623)
         at clojure.core$load__4911.doInvoke(core.clj:3622)
         at clojure.lang.RestFn.invoke(RestFn.java:413)
         at clojure.core$load_one__4863.invoke(core.clj:3467)
         at clojure.core$compile__4918$fn__4920.invoke(core.clj:3633)
         at clojure.core$compile__4918.invoke(core.clj:3632)
         at clojure.lang.Var.invoke(Var.java:336)
         at clojure.lang.Compile.main(Compile.java:56)


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

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

Comment by Assembla Importer [ 24/Aug/10 6:45 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)





[CLJ-790] Primitive type hints on function names should print error message Created: 10/May/11  Updated: 23/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Alan Dipert Assignee: Alan Dipert
Resolution: Unresolved Votes: 0
Labels: errormsgs


 Description   

Functions returning primitives are hinted with metadata on the argument list, not on the function name. Using a primitive type hint on a function name should print an error message.

Currently, misplaced primitive hints are read without error.



 Comments   
Comment by Andy Fingerhut [ 23/Feb/15 9:20 PM ]

One can type hint a primitive value on a Var naming a function, or any value one wants, like so:

(def {:tag 'long} foo 17)

(defn {:tag 'double} bar [x y]
(* 2.0 x y))

I think it is odd that one must use {:tag 'long} instead of ^long, since trying to use ^long ends up giving the useless type hint that is the value of the function clojure.core/long.

However, the Clojure compiler will use the primitive type hints as shown in the examples above to avoid reflection in appropriate Java interop calls, so making them an error seems undesirable.





[CLJ-995] sorted-set doesn't support IEditableCollection Created: 13/May/12  Updated: 24/Feb/15

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

Type: Enhancement Priority: Major
Reporter: Moritz Ulrich Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: collections


 Description   

I think sorted-set (PersistentTreeSet) should implement the transient interface. It's a special-purpose set and should be usable just like every normal set.



 Comments   
Comment by Michel Alexandre Salim [ 04/Jun/12 2:32 AM ]

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

Comment by Andy Fingerhut [ 24/Feb/15 10:00 AM ]

Anyone wishing for sorted sets/maps with transients should consider the data.avl library, which has them: https://github.com/clojure/data.avl





[CLJ-1599] Add a reset! that returns old value Created: 24/Nov/14  Updated: 02/Mar/15

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

Type: Enhancement Priority: Major
Reporter: Steven Yi Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: atom

Attachments: File get-and-set.diff    
Patch: Code
Approval: Triaged

 Description   

DESCRIPTION

This patch adds the equivalent of reset!, here called get-and-set!, to core to allow getting the last value from an atom and setting it to a new value. This is useful for atomically draining an atom of its value and setting to a new value. The implementation delegates to Java's AtomicReference.getAndSet().

The equivalent operation in Clojure code would be:

(defn get-and-set! [atm newv]
(loop [oldv @atm]
(if (compare-and-set! atm oldv newv)
oldv
(recur @atm))))

This is close to a 1:1 translation of the Java code in sun.misc.Unsafe's getAndSetObject, used by AtomicReference (as of current JDK9 source code).

APPLICATIONS

  • User may want to check if an operation has occurred before by using an atom as a flag. I.e.,

(def has-run-once (atom false))
...
(when-not (get-and-set! has-run-once true)
(do something))

  • User may want to use an atom similarly to a java.util.concurrent.LinkedTransferQueue, for the case of pairing up adds by writers and drainTo by readers:

Thread 1: (swap! atm conj item1)
Thread 2: (swap! atm conj item2)
Thread 3: (let [new-vals (get-and-set! atm [])]
(do-something new-vals))

ALTERNATIVES

  • For case of atom as flag, user can use existing compare-and-set!:

(def has-run-once (atom false))
...
(when-not (compare-and-set! has-run-once false true)
(do something))

Argument: get-and-set! is a little clearer in intent as it is using the value of the atom, rather than the success of the cas operation. Also, this would not be applicable to situations where the value stored is not a boolean.

  • User can just go ahead and use LinkedTransferQueue.

Argument: User not fluent in Java may not be readily able to use this.

==

Argument for: This seems like a sufficiently primitive operation to include in core for atoms. I am unsure of the rationale, but assume it was vetted to include into Java's AtomicReference for a reason. Also, if users are using atoms and have this available, they are less likely to try to do this incorrectly, such as:

(let [vals @some-atom]
(reset! some-atom [])
(do-something-with vals))

Argument against: This may not be sufficiently primitive enough to include in core. Users have a workaround to implement the get-and-set! operation in user-code as given above.

Note: This request is similar to CLJ-1454 (http://dev.clojure.org/jira/browse/CLJ-1454), but differs in that this is not a swap! operation that accepts an IFn argument. Also, I looked to add a test in test/clojure/test_clojure/atoms.clj, but saw that the other operations weren't tested. (I assume this is due to the other operations delegating to AtomicReference and hence not deemed test-worthy.)



 Comments   
Comment by Michael Blume [ 02/Mar/15 5:03 PM ]

I tend to wind up inevitably adding this to my projects, be nice to have it in core

Comment by Alex Miller [ 02/Mar/15 7:27 PM ]

So CLJ-1454 is swap! and return old and CLJ-1599 is reset! and return old?

Comment by Steven Yi [ 02/Mar/15 7:48 PM ]

Yes, I think that's an accurate interpretation of the two tickets.





[CLJ-1674] Boolean return type-hint confusing the compiler Created: 12/Mar/15  Updated: 12/Mar/15

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

Type: Defect Priority: Major
Reporter: Mikkel Kamstrup Erlandsen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: typehints
Environment:

OSX, Clojure 1.6.0



 Description   

Saving the below snippet and running it like

java -jar clojure-1.6.0.jar snippet.clj

Produces

$ java -jar clojure-1.6.0.jar snippet.clj 
Exception in thread "main" java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$boolean@1356d4d4, compiling:(/Users/kamstrup/tmp/snippet.clj:15:1)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6651)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6632)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.access$100(Compiler.java:38)
	at clojure.lang.Compiler$DefExpr$Parser.parse(Compiler.java:538)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6644)
	at clojure.lang.Compiler.analyze(Compiler.java:6445)
	at clojure.lang.Compiler.analyze(Compiler.java:6406)
	at clojure.lang.Compiler.eval(Compiler.java:6707)
	at clojure.lang.Compiler.load(Compiler.java:7130)
	at clojure.lang.Compiler.loadFile(Compiler.java:7086)
	at clojure.main$load_script.invoke(main.clj:274)
	at clojure.main$script_opt.invoke(main.clj:336)
	at clojure.main$main.doInvoke(main.clj:420)
	at clojure.lang.RestFn.invoke(RestFn.java:408)
	at clojure.lang.Var.invoke(Var.java:379)
	at clojure.lang.AFn.applyToHelper(AFn.java:154)
	at clojure.lang.Var.applyTo(Var.java:700)
	at clojure.main.main(main.java:37)
Caused by: java.lang.IllegalArgumentException: Unable to resolve classname: clojure.core$boolean@1356d4d4
	at clojure.lang.Compiler$HostExpr.tagToClass(Compiler.java:1069)
	at clojure.lang.Compiler$InvokeExpr.getJavaClass(Compiler.java:3659)
	at clojure.lang.Compiler$LocalBinding.hasJavaClass(Compiler.java:5657)
	at clojure.lang.Compiler$LocalBindingExpr.hasJavaClass(Compiler.java:5751)
	at clojure.lang.Compiler.maybePrimitiveType(Compiler.java:1283)
	at clojure.lang.Compiler$IfExpr.doEmit(Compiler.java:2631)
	at clojure.lang.Compiler$IfExpr.emit(Compiler.java:2613)
	at clojure.lang.Compiler$BodyExpr.emit(Compiler.java:5826)
	at clojure.lang.Compiler$LetExpr.doEmit(Compiler.java:6180)
	at clojure.lang.Compiler$LetExpr.emit(Compiler.java:6133)
	at clojure.lang.Compiler$BodyExpr.emit(Compiler.java:5826)
	at clojure.lang.Compiler$FnMethod.doEmit(Compiler.java:5374)
	at clojure.lang.Compiler$FnMethod.emit(Compiler.java:5232)
	at clojure.lang.Compiler$FnExpr.emitMethods(Compiler.java:3771)
	at clojure.lang.Compiler$ObjExpr.compile(Compiler.java:4410)
	at clojure.lang.Compiler$FnExpr.parse(Compiler.java:3904)
	at clojure.lang.Compiler.analyzeSeq(Compiler.java:6642)
	... 19 more

The snippet:

snippet.clj
;; Bug in the Clojure compiler (1.6.0): If we annotate the return here with ^boolean we get:
;; 'IllegalArgumentException: Unable to resolve classname: clojure.core$boolean' from the compiler.
;; Removing it, everything is as expected
(defn ^boolean foo-bar?
  [node]
  (= node "foo-bar"))

;; Check it out, we can have ^boolean here, but not on foo-bar? !! :-)
(defn ^boolean bar-foo?
  [node]
  (= node "bar-foo"))

;; Instead of removing the ^boolean return on foo-bar? we can also remove this function
;; to have all work as expected
(defn ^boolean interesting?
  [node]
  (or (foo-bar? node) (bar-foo? node)))

(println "Foo-Bar?" (foo-bar? "baz"))


 Comments   
Comment by Mikkel Kamstrup Erlandsen [ 12/Mar/15 5:25 AM ]

Typo in comment 2 in the snippet: s/xtc-scenario?/foo-bar?/

Comment by Nicola Mometto [ 12/Mar/15 6:01 AM ]

Metadata on def's symbol is evaluated as per the doc (http://clojure.org/special_forms), evaluating `boolean` results in the clojure.core/boolean function which is not a valid type hint.

As a rule of thumb, attach the return tag in the argvec rather than on the def symbol, in this case you should write

(defn foo-bar?
   ^boolean [node]
  (= node "foo-bar"))

I understand why the fact that

(defn ^boolean foo [] true)

and

(defn foo ^boolean [] true)

behave differently and the fact that the compiler will throw iff the type hint is used rather than throwing at the function definition time is confusing (and I've complained about this and the lack of documentation/specification regarding type hints for a while) but this is not a bug

Comment by Mikkel Kamstrup Erlandsen [ 12/Mar/15 6:36 AM ]

Thanks for clarifying Nicola, you are indeed correct.

Putting return type annotations before the method name seems to be common practice in a lot of Clojure code I've read online. Perpetuated by some online tutorials, and the clojure.org docs them selves (fx.

(defn ^:private ^String my-fn ...)
is found in http://clojure.org/cheatsheet)

Comment by Andy Fingerhut [ 12/Mar/15 8:36 AM ]

Mikkel: If the type tags are Java classes, not primitives, then ^Classname is a correct type tag. If you use Eastwood, it can warn about these incorrect type tags, and has some documentation on what works and what does not here: https://github.com/jonase/eastwood#wrong-tag

Also here: https://github.com/jonase/eastwood#unused-meta-on-macro





[CLJ-1679] Add fast path in seq comparison for structurally sharing seqs Created: 21/Mar/15  Updated: 23/Mar/15

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, seq

Attachments: Text File 0001-CLJ-1679-do-pointer-checks-in-seq-equality.patch     Text File CLJ-1679-v2.patch     Text File CLJ-1679-v3.patch    
Patch: Code and Test

 Description   

Currently comparing two non identical seqs requires iterating through both seqs comparing value by value, ignoring the possibility of seq `a` and `b` having the same (pointer-equal) rest.

The proposed patch adds a pointer equality check on the seq tails that can make the equality short-circuit if the test returns true, which is helpful when comparing large (or possibly infinite) seqs that share a common subseq.

After this patch, comparisons like

(let [x (range)] (= x (cons 0 (rest x))))
which currently don't terminate, return true in constant time.

Patch: CLJ-1679-v3.patch



 Comments   
Comment by Michael Blume [ 23/Mar/15 12:52 PM ]

When this test fails (it fails on my master, but I've got a bunch of other development patches, I'm still figuring out where the conflict is), it fails by hanging forever. Maybe it'd be better to check equality in a future and time out the future?

Comment by Michael Blume [ 23/Mar/15 1:01 PM ]

like so =)

Comment by Nicola Mometto [ 23/Mar/15 1:11 PM ]

Makes sense, thanks for the updated patch

Comment by Michael Blume [ 23/Mar/15 1:24 PM ]

Hm, previous patch had a problem where the reporting logic still tries to force the sequence and OOMs, this patch prevents that.

Comment by Michael Blume [ 23/Mar/15 1:41 PM ]

ok, looks like CLJ-1515, CLJ-1603, and this patch, all combine to fail together, though any two of them work fine.

Comment by Michael Blume [ 23/Mar/15 1:43 PM ]

(And really there's nothing wrong with the source of this patch, it still works nicely to short-circuit = where there's structural sharing, it's just that the other two patches break structural sharing for ranges, so the test fails)

Comment by Nicola Mometto [ 23/Mar/15 2:02 PM ]

I see, I guess we'll have to change the test if the patches for those tickets get applied.





[CLJ-666] Add support for Big* numeric types to Reflector Created: 29/Oct/10  Updated: 27/Jul/13

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

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

Attachments: Text File 0001-Add-Big-support-to-Reflector.patch     Text File 0001-Add-Big-support-to-Reflector-Updated.patch    
Patch: Code and Test

 Description   

This should work as expected, for example:

(Integer. 1N)

Probably for BigInt, BigInteger, and BigDecimal.

Method to look at is c.l.Reflector.paramArgTypeMatch, per Rich in irc.



 Comments   
Comment by Colin Jones [ 30/Mar/11 11:52 PM ]

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

Comment by Alexander Taggart [ 31/Mar/11 12:42 AM ]

Patch on CLJ-445 fixes this as well.

Comment by Colin Jones [ 27/Apr/11 4:41 PM ]

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.

Comment by Alexander Taggart [ 27/Apr/11 5:26 PM ]

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.

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

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

Comment by Chas Emerick [ 28/Apr/11 9:41 AM ]

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…

Comment by Luke VanderHart [ 29/Apr/11 10:01 AM ]

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.

Comment by Alex Ott [ 25/Jun/12 7:19 AM ]

Modified version of original patch

Comment by Andy Fingerhut [ 26/Jun/12 1:38 PM ]

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.

Comment by Alex Ott [ 28/Jun/12 1:00 PM ]

Renamed updated patch to unique name





[CLJ-140] Single :tag for type hints conflates value's type with type of return value from an invoke Created: 01/Jul/09  Updated: 03/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Anonymous Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: typehints


 Description   

The value of a Var can be operated on directly, or, if it is a fn, it can be invoked and the resulting value operated on. :tag metadata on a Var is used to provide a type hint to the compiler to avoid reflection. Having a single metadata key for this two distinct uses makes it possible (even easy, if unlikely) to create a situation where type-hinting the value causes a ClassCastException on an operation on the invocation return value, or the reverse. The only obvious solution is two use different keys for the two uses.



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

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





[CLJ-1459] records should support transient Created: 05/Jul/14  Updated: 06/Jul/14

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

Type: Enhancement Priority: Major
Reporter: Yongqian Li Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: defrecord


 Description   

user=> (defrecord R [a])
user.R
user=> (transient (->R nil))
ClassCastException user.R cannot be cast to clojure.lang.IEditableCollection clojure.core/transient (core.clj:3060)






[CLJ-1690] Make Range, Repeat and Cycle implement Indexed Created: 31/Mar/15  Updated: 01/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1690.patch    

 Description   

Currently, Cycle, Range and Repeat do not implement Indexed, which means that "nth" is O( n ) on average.

The proposed change is to implement Indexed for these classes, so that "nth" becomes an O( 1 ) operation.



 Comments   
Comment by Alex Miller [ 31/Mar/15 9:37 PM ]

This is an expansion of capabilities and commitments beyond what these functions have done in the past. We've already committed to more than we really wanted to with them, so I'm not sure we want to add yet more commitments. In any case, we won't do it for 1.7.

FYI, that Range you're patching is not currently used for anything - the current impl uses a chunked seq definition in core.clj. CLJ-1515 will (likely) replace the Range class with an all new impl. In any case, patching Range here probably isn't useful until CLJ-1515 is resolved.

Comment by Mike Anderson [ 31/Mar/15 9:50 PM ]

Understood re 1.7. Though I personally think this is small enough that you could squeeze it in. Your call.

However I still think this approach is useful though: nth is a very common operation and I note you aren't benchmarking it yet in CLJ-1515. Whatever new Range implementation is used will benefit from implementing Indexed.

Comment by Alex Miller [ 31/Mar/15 11:22 PM ]

None of these functions currently promises to return something Indexed. If we add that and people come to rely on it, we can never change the implementation in a way that removes it. So I'm not sure that's a promise we want to make.

Comment by Mike Anderson [ 01/Apr/15 1:39 AM ]

I am not proposing that we make any "promise" of an indexed return value, simply that such classes implement the interface as an implementation detail (where it makes sense).

This then causes the fast path in functions like RT.nth to be taken, so we get O(1) instead of O( n) for the most common indexed lookup cases.

TBH, my assumption was that this was the main purpose of the "Indexed" interface, i.e. to allow concrete collection types to participate in Clojure's indexed access functions with an efficient implementation.

Comment by Ghadi Shayban [ 01/Apr/15 10:25 AM ]

People regularly rely on implementation details, promise or no promise. If clojure were to add Indexed then remove it, people's code would either break or get slower.

Implementation behavior (whether overt or implicit) is necessarily treated as future constraints (shackles), so it is considered carefully.





[CLJ-308] protocol-ize with-open Created: 21/Apr/10  Updated: 28/Dec/14

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

Type: Enhancement Priority: Major
Reporter: Assembla Importer Assignee: Unassigned
Resolution: Unresolved Votes: 8
Labels: io

Attachments: Text File 0001-Added-ClosableResource-protocol-for-with-open.patch    
Patch: Code
Approval: Triaged

 Description   

Good use (and documentation example) of protocols: make with-open aware of a Closable protocol for APIs that use a different close convention. See http://groups.google.com/group/clojure/browse_thread/thread/86c87e1fc4b1347c



 Comments   
Comment by Assembla Importer [ 24/Aug/10 4:39 PM ]

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

Comment by Tassilo Horn [ 23/Dec/11 5:11 AM ]

Added a CloseableResource protocol and extended it on java.io.Closeable (implemented by all Readers, Writers, Streams, Channels, Sockets). Use it in with-open.

All tests pass.

Comment by Tassilo Horn [ 23/Dec/11 7:14 AM ]

Seems to be related to Scopes (http://dev.clojure.org/jira/browse/CLJ-2).

Comment by Tassilo Horn [ 08/Mar/12 3:59 AM ]

Updated patch.

Comment by Andy Fingerhut [ 02/Apr/12 12:11 PM ]

Patch 0001-Added-ClosableResource-protocol-for-with-open.patch dated 08/Mar/12 applies, builds, and tests cleanly on latest master as of Apr 2 2012. Tassilo has signed a CA.

Comment by Tassilo Horn [ 13/Apr/12 11:23 AM ]

Updated patch to apply cleanly against master again.

Comment by Brandon Bloom [ 22/Jul/14 9:00 PM ]

I looked up this ticket because I ran in to a reflection warning: with-open does not hint it's binding with java.io.Closeable

Some feedback on the patch:

1) This is a breaking change for anyone relying on the close method to be duck-typed.

2) CloseableResource is a bit long. clojure.core.protocols.Closeable is plenty unambiguous.

3) Rather than extending CloseableResource to java.io.Closeable, you can use the little known (undocumented? unsupported?) :on-interface directive:

(defprotocol Closeable
  :on-interface java.io.Closeable
  (close [this]))

That would perform much better than the existing patch.

Comment by Tassilo Horn [ 23/Jul/14 7:12 AM ]

Hi Brandon, two questions:

Could 1) be circumvented somehow by providing a default implementation somehow? I guess the protocol could be extended upon Object with implementation (.close this), but that would give a reflection warning since Object has no close method. Probably one could extend upon Object and in the implementation search a "close" method using java.lang.reflect and throw an exception if none could be found?

Could you please tell me a bit more about the :on-interface option? How does that differ from extend? And how do I add the implementation, i.e., (.close this) with that option?





[CLJ-1707] conditional form is not consumed when :read-allow is falsey Created: 15/Apr/15  Updated: 15/Apr/15

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reader


 Description   
user=> (def a (java.io.PushbackReader. (java.io.StringReader. "#?(:clj [1 2])")))
#'user/a
user=> (read a)
RuntimeException Conditional read not allowed  clojure.lang.Util.runtimeException (Util.java:221)
user=> (read a)
(:clj [1 2])

the expected result would be an EOF exception on the second read.






[CLJ-1117] partition docstring should be more explicit about dropped or partial trailing elements Created: 29/Nov/12  Updated: 11/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Timothy Baldridge Assignee: Unassigned
Resolution: Unresolved Votes: 6
Labels: docstring
Environment:

OS X, 10.8


Attachments: Text File clj-1117.patch    
Patch: Code
Approval: Triaged

 Description   

The doc for partition states "In case there are not enough padding elements, return a partition with less than n items." However, the behavior of this function is as follows:

user=> (partition 3 (range 10))
((0 1 2) (3 4 5) (6 7 8))
user=> (partition 4 (range 10))
((0 1 2 3) (4 5 6 7))

Proposed: The docstring should be updated to make it clear that not providing a pad means that items are dropped, and to also see partition-all.

Patch: clj-1117.patch



 Comments   
Comment by Andy Fingerhut [ 29/Nov/12 2:15 PM ]

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

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

Comment by Timothy Baldridge [ 29/Nov/12 2:55 PM ]

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

Comment by Andy Fingerhut [ 29/Nov/12 4:43 PM ]

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

Comment by Gabriel Horner [ 17/May/13 10:14 AM ]

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

Comment by Andrea Richiardi [ 11/Apr/15 10:38 AM ]

I was a little bit baffled by this as well...in particular, I could not figure out why (partition 2 3 [1 2 3 4 5 6 7]) is not '((1 2) (4 5) (7))...it would really be better to rewrite that comment.





[CLJ-1701] Serialization of protocol methods broken: java.io.NotSerializableException: clojure.lang.MethodImplCache Created: 13/Apr/15  Updated: 20/Apr/15

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

Type: Defect Priority: Major
Reporter: Dr. Christian Betz Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

With this test, you can see that we cannot serialize methods from protocols (i.e. time-from-tweet), as this results in a java.io.NotSerializableException: clojure.lang.MethodImplCache
at java.io.ObjectOutputStream.writeObject0 (ObjectOutputStream.java:1183)
java.io.ObjectOutputStream.defaultWriteFields (ObjectOutputStream.java:1547)
java.io.ObjectOutputStream.writeSerialData (ObjectOutputStream.java:1508)
java.io.ObjectOutputStream.writeOrdinaryObject (ObjectOutputStream.java:1431)
java.io.ObjectOutputStream.writeObject0 (ObjectOutputStream.java:1177)
java.io.ObjectOutputStream.writeObject (ObjectOutputStream.java:347)
sparkling.protocol_test$serialize.invoke (protocol_test.clj:11)

This is the actual test:

(ns sparkling.protocol-test
(:require [clojure.test :refer :all])
(:import [java.io ObjectInputStream ByteArrayInputStream ObjectOutputStream ByteArrayOutputStream]))

(defn- serialize
"Serializes a single object, returning a byte array."
[v]
(with-open [bout (ByteArrayOutputStream.)
oos (ObjectOutputStream. bout)]
(.writeObject oos v)
(.flush oos)
(.toByteArray bout)))

(defn- deserialize
"Deserializes and returns a single object from the given byte array."
[bytes]
(with-open [ois (-> bytes ByteArrayInputStream. ObjectInputStream.)]
(.readObject ois)))

(defprotocol timestamped
(time-from-tweet [item]))

(defrecord tweet [username tweet timestamp]
timestamped
(time-from-tweet [_]
timestamp
))

(deftest sequable-serialization
(testing "Serialization of function"
(let [item identity]
(is item (-> item serialize deserialize))))

(testing "Serialization of protocol method"
(let [item time-from-tweet]
(is item (-> item serialize deserialize)))))



 Comments   
Comment by Dr. Christian Betz [ 13/Apr/15 6:15 AM ]

BTW: Same is true for multimethods, here the exception is java.io.NotSerializableException: clojure.lang.MultiFn

Comment by Alex Miller [ 13/Apr/15 9:35 AM ]

I don't think we expect functions to be serializable in this way. Both protocols and multimethods effectively have runtime state based on what implementations have extended them. What would it mean to serialize these functions? Would you serialize them with whatever implementations have been loaded at that point? Or with none? Both seem problematic to me. Regular functions are closures and can capture the state of their environment. I think better answers are either AOT or for regular functions, something like the serializable-fn library.

Comment by Dr. Christian Betz [ 13/Apr/15 1:13 PM ]

Hi,

thanks for the comments. First, something to the background: I'm developing Sparkling, a Clojure API to Apache Spark. For distributing code in the cluster it depends on AOT compiled functions, so yes, you cannot simply serialize any function around, it needs to be AOT'd. Serializiation provides us with support for the current bindings etc, and everything works as expected. So, AFunction is serializable for a reason and so are other implementations of AFn/IFn, everything works well.

Regarding the state of protocols and multimethods - I think it's conceptually the same as the state of functions (which function definition, the var might be bound multiple times, etc.), and the closures given in bindings. There's no reason for me as the user of a protocol to believe that the method from the protocol differs from a function. In fact (ifn? protocol-method) also returns true.

serializable-fn, not being intended for over-the-wire serialization in the first place, has problems with collections of functions in bindings of the serializble function, together with an issue with PermGen pollution by creating classes for the same function over and over again in the context of Spark.

I think I'm fine for the moment, as I can wrap the protocol method in a function, but I still believe, that this is a bug.

Regards

Chris

Comment by Dr. Christian Betz [ 20/Apr/15 3:10 AM ]

actually, this is the code snippet from clojure.lang.AFunction causing the pain:

clojure.lang.AFunction.java
public abstract class AFunction extends AFn implements IObj, Comparator, Fn, Serializable {

public volatile MethodImplCache __methodImplCache;

AFunction is serializable, but MethodImplCache is not. I'm not sure if it's enough to mark it as transient, because I did not check where initialization happens.

Comment by Dr. Christian Betz [ 20/Apr/15 3:29 AM ]

My comment per mail got lost in SMTP-nirvana: There's an easy workaround. Wrap the protocol method in a function, that will do the trick at the cost of uglifying your code





[CLJ-1386] Add transient? predicate Created: 17/Mar/14  Updated: 20/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Devin Walters Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: collections, transient
Environment:

N/A


Attachments: Text File 0004-Add-transient-predicate.patch    
Patch: Code and Test
Approval: Triaged

 Description   

I've encountered situations where I wanted to check whether something was transient in order to know whether I should call assoc! or assoc, conj! or conj, etc.

This patch adds `transient?` as a predicate fn.



 Comments   
Comment by Alex Miller [ 17/Mar/14 10:21 AM ]

Patch needs a docstring and a test.

Comment by Devin Walters [ 17/Mar/14 4:42 PM ]

Alex: I figured that would be the case! Sorry about that. I've updated the patch. It now includes a docstring and has tests of `transient?` for #{}, [], and {}.

Thanks!

Comment by Alex Miller [ 17/Mar/14 9:48 PM ]

Thanks - please don't use the labels "patch" or "test" - those are covered by the Patch field.

Comment by Devin Walters [ 18/Mar/14 9:17 AM ]

Ah, sorry for the mixup Alex. I assumed you removed "patch" as a label the first time around to flag this ticket as still needing a vetted patch. My mistake.

Comment by Andy Fingerhut [ 21/Mar/14 1:42 PM ]

Patch 0001-Add-transient-predicate.patch dated Mar 17, 2014 applies cleanly to latest Clojure master, but fails a test because the new function transient? has no :added metadata. See most other Clojure functions in clojure.core for examples.

It also generates a new warning while running tests:

WARNING: transient? already refers to: #'clojure.core/transient? in namespace: clojure.test-clojure.data-structures, being replaced by: #'clojure.test-clojure.data-structures/transient?

There is an older (but equivalent) definition of transient? in test file data_structures.clj that should be removed when adding it to clojure.core

Comment by Devin Walters [ 22/Mar/14 11:29 PM ]

@Andy, the reason I did not add :added metadata is because I do not know if/when this patch will be accepted, and as a result, I don't really know if it will sneak into 1.6.X or 1.7. For now, I've put it in as 1.7. If it's in the running to be added sooner than that, let me know and I'll adjust it.

RE: The warning. Good catch. I've submitted a new patch which removes the private version of transient? from data_structures.clj. All tests pass.

Edit to Add: The latest patch as of this comment is now 0002-Add-transient-predicate.patch.

Comment by Andy Fingerhut [ 06/Aug/14 2:16 PM ]

Patch 0002-Add-transient-predicate.patch dated Mar 22 2014 no longer applies cleanly to latest Clojure master due to some changes committed earlier today. I haven't checked whether this patch is straightforward to update.

Comment by Devin Walters [ 06/Aug/14 4:11 PM ]

I've updated the patch to 0003-Add-transient-predicate.patch. This patch applies cleanly to the latest version of master.

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

Patch 0003-Add-transient-predicate.patch dated Aug 6 2013 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.

Comment by Devin Walters [ 31/Aug/14 12:01 AM ]

I've updated the patch to 0004-Add-transient-predicate.patch. This patch applies cleanly to the latest version of master.





[CLJ-1149] Unhelpful error message from :use (and use function) when arguments are malformed Created: 17/Jan/13  Updated: 28/Sep/13

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

Type: Enhancement Priority: Major
Reporter: Sean Corfield Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: errormsgs

Approval: Triaged

 Description   

the following exception happens when you have something like this(bad):

(ns runtime.util-test
(:use [midje.sweet :reload-all]))

as opposed to any of these(correct):

(ns runtime.util-test
(:use midje.sweet :reload-all))

(ns runtime.util-test
(:use [midje.sweet] :reload-all))

and the exception is:
Exception in thread "main" java.lang.IllegalArgumentException: No value supplied for key: true
at clojure.lang.PersistentHashMap.create(PersistentHashMap.java:77)
at clojure.core$hash_map.doInvoke(core.clj:365)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:617)
at clojure.core$load_lib.doInvoke(core.clj:5352)
at clojure.lang.RestFn.applyTo(RestFn.java:142)
at clojure.core$apply.invoke(core.clj:619)
at clojure.core$load_libs.doInvoke(core.clj:5403)
at clojure.lang.RestFn.applyTo(RestFn.java:137)
at clojure.core$apply.invoke(core.clj:621)
at clojure.core$use.doInvoke(core.clj:5497)

Note that this is similar to the equally unhelpful message shown in http://dev.clojure.org/jira/browse/CLJ-1140 although that is a different root cause.

Probably best to enhance the `use` function to validate its arguments before trying to apply hash-map?



 Comments   
Comment by Gary Fredericks [ 26/May/13 3:17 PM ]

I believe this applies to require as well.





[CLJ-992] `iterate` reducer Created: 10/May/12  Updated: 29/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Alan Malloy Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: reducers

Attachments: Text File 0001-Add-reducers-iterate.patch     Text File iterate-reducer.patch    
Patch: Code and Test
Approval: Vetted

 Description   

Added a reducer implementation mirroring clojure.core/iterate.

Patch: iterate-reducer.patch

Screened by:



 Comments   
Comment by Alan Malloy [ 10/May/12 9:50 PM ]

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.

Comment by Alan Malloy [ 10/May/12 10:24 PM ]

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

Comment by Jason Jackson [ 11/May/12 11:23 AM ]

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?

Comment by Alan Malloy [ 18/Aug/12 7:16 PM ]

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.





[CLJ-1351] Clojure emits an unused "swapThunk" method for functions with keyword callsites Created: 14/Feb/14  Updated: 29/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, ft, performance

Attachments: Text File 0001-remove-unused-swapThunk-method-generation.patch    
Patch: Code
Approval: Triaged

 Description   

This method is no longer used, I did a quick git blame and it look like it was used for an earlier implementation of keyword callsites and forgot to be removed in this commit https://github.com/clojure/clojure/commit/c7af275d4ee33cdc1794c8df8fa1e6d39039ac84

Removing this should reduce a bit the size of compile functions.

Screened by: Alex Miller






[CLJ-1295] Speed up dissoc on array-maps Created: 15/Nov/13  Updated: 29/Apr/15

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

Type: Enhancement Priority: Major
Reporter: Andy Fingerhut Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: ft, performance

Attachments: File clj-1295-1.diff    
Patch: Code
Approval: Triaged

 Description   

In latest Clojure master as of Nov 15 2013, the method without() in PersistentArrayMap.java first searches