<< Back to previous view

[CLJ-1499] Replace seq-based iterators with direct iterators for all non-seq collections that use SeqIterator Created: 08/Aug/14  Updated: 07/Oct/14

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

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

Attachments: File clj-1499-all.diff     File clj-1499-v2.diff     File clj-1499-v3.diff     Text File defrecord-iterator.patch     File defrecord-iterator-v2.diff    
Patch: Code and Test
Approval: Vetted

 Description   

Add support for direct iterators instead of seq-based iterators for non-seq collections that use SeqIterator.

Patch adds support for direct iterators on the following (removing use of SeqIterator):

  • PersistentHashMap - new internal iterator (~20% faster)
  • APersistentSet - use internal map impl iterator (~5% faster)
  • PersistentQueue
  • PersistentStructMap
  • records (in core_deftype.clj)

Patch does not change use of SeqIterator in:

  • LazyTransformer$MultiStepper (not sure if this could be changed)
  • ASeq
  • LazySeq

Patch: clj-1499-all.diff



 Comments   
Comment by Alex Miller [ 13/Aug/14 1:57 PM ]

The list of non-seqs that uses SeqIterator are:

  • records (in core_deftype.clj)
  • APersistentSet - fallback, maybe is ok?
  • PersistentHashMap
  • PersistentQueue
  • PersistentStructMap

Seqs (that do not need to be changed) are:

  • ASeq
  • LazySeq.java

LazyTransformer$MultiStepper - not sure

Comment by Ghadi Shayban [ 27/Sep/14 2:16 PM ]

attached iterator impl for defrecords. ready to leverage iteration for extmap when PHM iteration lands.

Comment by Alex Miller [ 29/Sep/14 12:52 PM ]

PHM patch

Comment by Alex Miller [ 29/Sep/14 10:26 PM ]

New patch that fixes bugs with PHMs with null keys (and added tests to expose those issues), added support for PHS.

Comment by Ghadi Shayban [ 29/Sep/14 10:45 PM ]

Alex, the defrecord patch already uses the iterator for extmap. It's just made better by the PHM patch.

Comment by Alex Miller [ 29/Sep/14 10:47 PM ]

Comment by Ghadi Shayban [ 30/Sep/14 4:17 PM ]

Heh. Skate to where the puck is going to be – Gretzky

Re: defrecord iterator: As is, it propagates exceptions from reaching the end of the ExtMap's iterator. As noted in CLJ-1453, PersistentArrayMap's iterator improperly returns an ArrayIndexOutOfBoundsException, rather than NoSuchElementException.

Comment by Alex Miller [ 30/Sep/14 6:41 PM ]

Hey Ghadi, rather than rebuilding the case map to pass to the RecordIterator, why don't you just pass the fields in iteration order to it and leverage the case map via .valAt like everything else?

Comment by Ghadi Shayban [ 30/Sep/14 7:30 PM ]

defrecord-iterator-v2.diff reuses valAt and minimizes macrology.

Comment by Alex Miller [ 07/Oct/14 2:04 PM ]

Comments from Stu (found under the couch):

"1. some of the impls (e.g. queue manually concatenate two iters. Would implementing iter-cat and calling that be simpler and more robust?
2. I found this tweak to the generative testing more useful in reporting failure, non-dependent on clojure.test, and capable of expecting failures. Waddya think?

(defn seq-iter-match
  [seqable iterable]
  (let [i (.iterator iterable)]
    (loop [s (seq seqable)
           n 0]
      (if (seq s)
        (do
          (when-not (.hasNext i)
            (throw (ex-info "Iterator exhausted before seq"
                            {:pos n :seqable seqable :iterable iterable})))
          (when-not (= (.next i) (first s))
            (throw (ex-info "Iterator and seq did not match"
                            {:pos n :seqable seqable :iterable iterable})))
          (recur (rest s) (inc n)))
        (when (.hasNext i)
          (throw (ex-info "Seq exhausted before iterator"
                          {:pos n :seqable seqable :iterable iterable})))))))

		(defspec seq-and-iter-match-for-maps
  identity
  [^{:tag clojure.test-clojure.data-structures/gen-map} m]
  (seq-iter-match m m))

3. similar generative approach would be good for the other types (looks like we just do maps)"





[CLJ-703] Improve writeClassFile performance Created: 04/Jan/11  Updated: 08/Oct/14

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: Unassigned
Resolution: Unresolved Votes: 19
Labels: Compiler, performance

Attachments: Text File 0001-use-File.mkdirs-instead-of-mkdir-every-single-direct.patch     Text File 0002-Ensure-atomic-creation-of-class-files-by-renaming-a-.patch     Text File improve-writeclassfile-perf.patch     Text File remove-flush-and-sync-only.patch     Text File remove-sync-only.patch    
Patch: Code
Approval: Triaged

 Description   

This Discussion about timing issues when writing class files led to the the current implementation of synchronous writes to disk. This leads to bad performance/system-load when compiling Clojure code.

This Discussion questioned the current implentation.

Synchronous writes are not necessary and also do not ensure (crash while calling write) valid classfiles.

These Patches (0001 is just a code cleanup for creating the directory structure) ensures atomic creation of classfiles by using File.renameTo()



 Comments   
Comment by David Powell [ 17/Jan/11 2:16 PM ]

Removing sync makes clojure build much faster. I wonder why it was added in the first place? I guess only Rich knows? I assume that it is not necessary.

If we are removing sync though, I wouldn't bother with the atomic rename stuff. Doing that sort of thing can cause problems on some platforms, eg with search indexers and virus checkers momentarily locking files after they are created.

The patch seems to be assuming that sync is there for some reason, but my initial assumption would be that sync isn't necessary - perhaps it was working around some issue that no longer exists?

Comment by Jürgen Hötzel [ 19/Jan/11 2:05 PM ]

Although its unlikely: there is a possible race condition "loading a paritally written classfile"?:

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/RT.java#L393

Comment by John Szakmeister [ 25/May/12 4:22 AM ]

The new improve-writeclassfile-perf version of the patch combines the two previous patches into a single patch file and brings them up-to-date with master. I can split the two changes back out into separate patch files if desired, but I figured out current tooling is more geared towards a single patch being applied.

Comment by John Szakmeister [ 25/May/12 4:36 AM ]

FWIW, both fixes look sane. The first one is a nice cleanup. The second one is a little more interesting in that uses a rename operation to put the final file into place. It removes the sync call, which does make things faster. In general, if we're concerned about on-disk consistency, we should really have a combination of the two: write the full contents to a tmp file, sync it, and atomically rename to the destination file name.

Neither the current master, nor the current patch will guarantee on-disk consistency across a machine wide crash. The current master could crash before the sync() occurs, leaving the file in an inconsistent state. With the patch, the OS may not get the file from file cache to disk before an OS level crash occurs, and leave the file in an inconsistent state as well. The benefit of the patch version is that the whole file does atomically come into view at once. It does have a nasty side effect of leaving around a temp file if the compiler crashes just before the rename though.

Perhaps a little more work to catch an exception and clean up is in order? In general, I like the patched version better.

Comment by Ivan Kozik [ 05/Oct/12 7:07 PM ]

File.renameTo returns false on (most?) errors, but the patch doesn't check for failure. Docs say "The return value should always be checked to make sure that the rename operation was successful." Failure might be especially likely on Windows, where files are opened by others without FILE_SHARE_DELETE.

Comment by Dave Della Costa [ 23/Jun/14 11:37 PM ]

We've been wondering why our compilation times on linux were so slow. It became the last straw when we walked away from one project and came back after 15 minutes and it was not done yet.

After some fruitless investigation into our linux configuration and lein java args, we stumbled upon this issue via the associated Clojure group thread. Upon commenting out the flush() and sync() lines (https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L7171-L7172) and compiling Clojure 1.6 ourselves, our projects all started compiling in under a minute.

Point being, can we at least provide some flag to allow for "unsafe compilation" or something? As it is, this is bad enough that we've manually modified all our local versions of Clojure to work around the issue.

Comment by Tim McCormack [ 30/Sep/14 4:10 PM ]

Additional motivation: This becomes really unpleasant on an encrypted filesystem, since write and read latency become higher.

As a partial workaround, I've been using this script to mount a ramdisk over top of target, which speeds up compilation 2-4x: https://gist.github.com/timmc/6c397644668bcf41041f (but removing flush() and sync() entirely would probably speed things up even more, if safe)

Comment by Ragnar Dahlen [ 06/Oct/14 12:30 PM ]

I'd like to explore this issue further as I also don't think the flush and sync calls add any value, but do have a severe impact on performance.

To resurrect the discussion, I've attached a new patch with the following approach:

  • create a temporary file
  • write class bytecode to temporary file, no flush or sync
  • close temporary file
  • atomic rename of temporary file to class file name

It is different to previous patches in that:

  • it applies cleanly to master
  • it checks return value from File.renameTo
  • it omits proposed File.mkdirs change as the current implementation is actually converting from an "internal name", where forward slashes are assumed (splits on "/"), to a platform specific path using File.separator. I'm not convinced that the previous patch is safe on all versions of Windows, and I think it's separate from the main issue here.

I opted for the atomic rename of a temp file to avoid leaving empty class files with a correct expected class file name in case of failure.

It is my understanding that this patch will guarantee that:

  • when writeClassFile returns successfully, a class file with the expected name will exists, and subsequent reads from that file name will return the expected bytecode (possibly from kernel buffers).
  • when writeClassFile fails, a class file with the expected name will not have been created (or updated if it previously existed).

Anything preventing the operating system from flushing its buffers to disk (power failure etc) might leave a corrupt (empty, partially written) class file. It's my opinion that that is acceptable behaviour, and worth the performance improvement (I'm seeing AOT compilation reduced from 1m20s -> 22s on one of our codebases, would be happy to provide more benchmarks if requested).

Would be grateful for feedback/testing of this patch.

Comment by Ragnar Dahlen [ 08/Oct/14 6:00 AM ]

We're testing this patch on various projects/platforms at my company. So far we've seen:

  • Significantly reduced compilation times on Linux (two typical examples: 30s to 15s, 1m30s to 30s)
  • No significant change in compilation times on Mac OSX.
  • File.renameTo consistently failing on a Windows machine.

My understanding is that the performance difference between Linux and OSX is due to differences in how these platforms implement fsync. OSX by default does not actually tell the drive to flush its buffers (requires fcntl F_FULLSYNC for this, not used by JVMs) [1], whereas Linux does [2].

Our very limited test shows (as was previously pointed out) that File.renameTo is problematic on Windows. I've attached a new patch that doesn't use rename, and only has the the sync call removed (flush is a no-op for FileOutputStreams). We're currently testing this patch.

The drawback of this patch is that it may leave correctly named, but empty class files if the write fails. One option would be to try and delete the file in the catch block. Personally, I wouldn't expect a compilation that failed because of OS/IO reasons to leave my classfiles in a consistent state.

[1]: "Note that while fsync() will flush all data from the host to the drive (i.e. the "permanent storage device"), the drive itself may not physically write the data to the platters for quite some time and it may be written in an out-of-order sequence.": https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man2/fsync.2.html

[2]: "[...] includes writing through or flushing a disk cache if present."
http://man7.org/linux/man-pages/man2/fsync.2.html





[CLJ-1152] PermGen leak in multimethods and protocol fns when evaled Created: 30/Jan/13  Updated: 06/Oct/14

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

Type: Defect Priority: Critical
Reporter: Chouser Assignee: Unassigned
Resolution: Unresolved Votes: 9
Labels: memory, protocols

Attachments: File naive-lru-for-multimethods-and-protocols.diff     File protocol_multifn_weak_ref_cache.diff    
Patch: Code
Approval: Incomplete

 Description   

There is a PermGen memory leak that we have tracked down to protocol methods and multimethods called inside an eval, because of the caches these methods use. The problem only arises when the value being cached is an instance of a class (such as a function or reify) that was defined inside the eval. Thus extending IFn or dispatching a multimethod on an IFn are likely triggers.

Reproducing: The easiest way that I have found to test this is to set "-XX:MaxPermSize" to a reasonable value so you don't have to wait too long for the PermGen spaaaaace to fill up, and to use "-XX:+TraceClassLoading" and "-XX:+TraceClassUnloading" to see the classes being loaded and unloaded.

leiningen project.clj
(defproject permgen-scratch "0.1.0-SNAPSHOT"
  :dependencies [[org.clojure/clojure "1.5.0-RC1"]]
  :jvm-opts ["-XX:MaxPermSize=32M"
             "-XX:+TraceClassLoading"
             "-XX:+TraceClassUnloading"])

You can use lein swank 45678 and connect with slime in emacs via M-x slime-connect.

To monitor the PermGen usage, you can find the Java process to watch with "jps -lmvV" and then run "jstat -gcold <PROCESS_ID> 1s". According to the jstat docs, the first column (PC) is the "Current permanent space capacity (KB)" and the second column (PU) is the "Permanent space utilization (KB)". VisualVM is also a nice tool for monitoring this.

Multimethod leak

Evaluating the following code will run a loop that eval's (take* (fn foo [])).

multimethod leak
(defmulti take* (fn [a] (type a)))

(defmethod take* clojure.lang.Fn
  [a]
  '())

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

In the lein swank session, you will see many lines like below listing the classes being created and loaded.

[Loaded user$eval15802$foo__15803 from __JVM_DefineClass__]
[Loaded user$eval15802 from __JVM_DefineClass__]

These lines will stop once the PermGen space fills up.

In the jstat monitoring, you'll see the amount of used PermGen space (PU) increase to the max and stay there.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 31616.0  31552.7    365952.0         0.0      4     0    0.000    0.129
 32000.0  31914.0    365952.0         0.0      4     0    0.000    0.129
 32768.0  32635.5    365952.0         0.0      4     0    0.000    0.129
 32768.0  32767.6    365952.0      1872.0      5     1    0.000    0.177
 32768.0  32108.2    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32470.4    291008.0     23681.8      6     2    0.827    1.006
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258
 32768.0  32767.2    698880.0     24013.8      8     4    1.073    1.258

A workaround is to run prefer-method before the PermGen space is all used up, e.g.

(prefer-method take* clojure.lang.Fn java.lang.Object)

Then, when the used PermGen space is close to the max, in the lein swank session, you will see the classes created by the eval'ing being unloaded.

[Unloading class user$eval5950$foo__5951]
[Unloading class user$eval3814]
[Unloading class user$eval2902$foo__2903]
[Unloading class user$eval13414]

In the jstat monitoring, there will be a long pause when used PermGen space stays close to the max, and then it will drop down, and start increasing again when more eval'ing occurs.

-    PC       PU        OC          OU       YGC    FGC    FGCT     GCT
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  32767.9    159680.0     24573.4      6     2    0.167    0.391
 32768.0  17891.3    283776.0     17243.9      6     2   50.589   50.813
 32768.0  18254.2    283776.0     17243.9      6     2   50.589   50.813

The defmulti defines a cache that uses the dispatch values as keys. Each eval call in the loop defines a new foo class which is then added to the cache when take* is called, preventing the class from ever being GCed.

The prefer-method workaround works because it calls clojure.lang.MultiFn.preferMethod, which calls the private MultiFn.resetCache method, which completely empties the cache.

Protocol leak

The leak with protocol methods similarly involves a cache. You see essentially the same behavior as the multimethod leak if you run the following code using protocols.

protocol leak
(defprotocol ITake (take* [a]))

(extend-type clojure.lang.Fn
  ITake
  (take* [this] '()))

(def stop (atom false))
(def sleep-duration (atom 1000))

(defn run-loop []
  (when-not @stop
    (eval '(take* (fn foo [])))
    (Thread/sleep @sleep-duration)
    (recur)))

(future (run-loop))

(reset! sleep-duration 0)

Again, the cache is in the take* method itself, using each new foo class as a key.

Workaround: A workaround is to run -reset-methods on the protocol before the PermGen space is all used up, e.g.

(-reset-methods ITake)

This works because -reset-methods replaces the cache with an empty MethodImplCache.

Patch: protocol_multifn_weak_ref_cache.diff

Screened by:



 Comments   
Comment by Chouser [ 30/Jan/13 9:10 AM ]

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

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

Comment by Jamie Stephens [ 18/Oct/13 9:35 AM ]

At a minimum, perhaps a switch to disable the caches – with obvious performance impact caveats.

Seems like expensive LRU logic is probably the way to go, but maybe don't have it kick in fully until some threshold is crossed.

Comment by Alex Miller [ 18/Oct/13 4:28 PM ]

A report seeing this in production from mailing list:
https://groups.google.com/forum/#!topic/clojure/_n3HipchjCc

Comment by Adrian Medina [ 10/Dec/13 11:43 AM ]

So this is why we've been running into PermGen space exceptions! This is a fairly critical bug for us - I'm making extensive use of multimethods in our codebase and this exception will creep in at runtime randomly.

Comment by Kevin Downey [ 17/Apr/14 9:52 PM ]

it might be better to split this in to two issues, because at a very abstract level the two issues are the "same", but concretely they are distinct (protocols don't really share code paths with multimethods), keeping them together in one issue seems like a recipe for a large hard to read patch

Comment by Kevin Downey [ 26/Jul/14 5:49 PM ]

naive-lru-method-cache-for-multimethods.diff replaces the methodCache in multimethods with a very naive lru cache built on PersistentHashMap and PersistentQueue

Comment by Kevin Downey [ 28/Jul/14 7:09 PM ]

naive-lru-for-multimethods-and-protocols.diff creates a new class clojure.lang.LRUCache that provides an lru cache built using PHashMap and PQueue behind an IPMap interface.

changes MultiFn to use an LRUCache for its method cache.

changes expand-method-impl-cache to use an LRUCache for MethodImplCache's map case

Comment by Kevin Downey [ 30/Jul/14 3:10 PM ]

I suspect my patch naive-lru-for-multimethods-and-protocols.diff is just wrong, unless MethodImplCache really is being used as a cache we can't just toss out entries when it gets full.

looking at the deftype code again, it does look like MethidImplCache is being used as a cache, so maybe the patch is fine

if I am sure of anything it is that I am unsure so hopefully someone who is sure can chime in

Comment by Nicola Mometto [ 31/Jul/14 11:02 AM ]

I haven't looked at your patch, but I can confirm that the MethodImplCache in the protocol function is just being used as a cache

Comment by dennis zhuang [ 08/Aug/14 6:21 AM ]

I developed a new patch that convert the methodCache in MultiFn to use WeakReference for dispatch value,and clear the cache if necessary.

I've test it with the code in ticket,and it looks fine.The classes will be unloaded when perm gen is almost all used up.

Comment by Alex Miller [ 22/Aug/14 4:55 PM ]

I don't know which to evaluate here. Does multifn_weak_method_cache.diff supersede naive-lru-for-multimethods-and-protocols.diff or are these alternate approaches both under consideration?

Comment by Kevin Downey [ 22/Aug/14 8:26 PM ]

the most straight forward thing, I think, is to consider them as alternatives, I am not a huge fan of weakrefs, but of course not using weakrefs we have to pick some bounding size for the cache, and the cache has a strong reference that could prevent a gc, so there are trade offs. My reasons to stay away from weak refs in general are using them ties the behavior of whatever you are building to the behavior of the gc pretty strongly. that may be considered a matter of personal taste

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

All patches dated Aug 8 2014 and earlier no longer applied cleanly to latest master after some commits were made to Clojure on Aug 29, 2014. They did apply cleanly before that day.

I have not checked how easy or difficult it might be to update the patches.

Comment by Kevin Downey [ 29/Aug/14 7:00 PM ]

I've updated naive-lru-for-multimethods-and-protocols.diff to apply to the current master

Comment by Andy Fingerhut [ 29/Aug/14 7:34 PM ]

Thanks, Kevin. While JIRA allows multiple attachments to a ticket with the same filename but different contents, that can be confusing for people looking for a particular patch, and for a program I have that evaluates patches for things like whether they apply and build cleanly. Would you mind removing the older one, or in some other way making all the names unique?

Comment by Kevin Downey [ 29/Aug/14 8:43 PM ]

I deleted all of my attachments accept for my latest and greatest

Comment by dennis zhuang [ 30/Aug/14 9:51 AM ]

I updated multifn_weak_method_cache2.diff patch too.

I think using weak reference cache is better,because we have to keep one cache per multifn.When you have many multi-functions, there will be many LRU caches in memory,and they will consume too much memory and CPU for evictions. You can't choose a proper threshold for LRU cache in every environment.
But i don't have any benchmark data to support my opinion.

Comment by Alex Miller [ 10/Sep/14 2:38 PM ]

I'm going to set the LRU cache patch aside. I don't think it's possible to find a "correct" size for it and it seems weird to me to extend APersistentMap to build such a thing anyways.

I think it makes more sense to follow the same strategy used for other caches (such as the Keyword cache) - a combination ConcurrentHashMap with WeakReferences and a ReferenceQueue for clean-up. I don't see any compelling reason not to take the same path as other internal caches.

Comment by Alex Miller [ 10/Sep/14 3:44 PM ]

Stepping back a little to think about the problem.... our requirements are:
1) cache map of dispatch value (could be any Object) to multimethod function (IFn)
2) do we want keys to be compared based on equality or identity? identity-based opens up more reference-based caching options and is fine for most common dispatch types (Class, Keyword), but reduces (often eliminates?) cache hits for all other types where values are likely to be equiv but not identical (vector of strings for example)
3) concurrent access to cache
4) cache cannot grow without bound
5) cache cannot retain strong references to dispatch values (the cache keys) because the keys might be instances of classes that were loaded in another classloader which will prevent GC in permgen

multifn_weak_method_cache.diff uses a ConcurrentHashMap (#3) that maps RefWrapper around keys to IFn (#1). The patch uses Util.equals() (#2) for (Java) equality-based comparisons. The RefWrapper wraps them in WeakReferences to avoid #5. Cache clearing based on the ReferenceQueue is used to prevent #4.

A few things definitely need to be fixed:

  • Util.equals() should be Util.equiv()
  • methodCache and rq should be final
  • Why does RefWrapper have obj and expect rq to possibly be null?
  • RefWrapper fields should all be final
  • Whitespace errors in patch

Another idea entirely - instead of caching dispatch value, cache based on hasheq of dispatch value then equality check on value. Could then use WeakHashMap and no RefWrapper.

This patch does not cover the protocol cache. Is that just waiting for the multimethod case to look good?

Comment by dennis zhuang [ 10/Sep/14 7:18 PM ]

Hi, alex, thanks for your review.But the latest patch is multifn_weak_method_cache2.diff. I will update the patch soon by your review, but i have a few questions to be explained.

1) I will use Util.equiv() instead of Util.equals().But what's the difference of them?
2) When the RefWrapper is retained as key in ConcurrentHashMap, it wraps the obj in WeakReference.But when trying to find it in ConcurrentHashMap, it uses obj directly as strong reference, and create it with passing null ReferenceQueue.Please look at the multifn_weak_method_cache2.diff line number 112. It short, the patch stores the dispatch value as weak reference in cache,but uses strong reference for cache getting.

3) If caching dispatch value based on hasheq , can we avoid hasheq value conflicts? If two different dispatch value have a same hasheq( or why it doesn't happen?), we would be in trouble.

Sorry, the patch doesn't cover the protocol cache, i will add it ASAP.

Comment by dennis zhuang [ 11/Sep/14 2:02 AM ]

The new patch 'protocol_multifn_weak_ref_cache.diff' is uploaded.

1) Using Util.equiv() instead of Util.equals()
2) Moved the RefWrapper and it's associated methods to Util.java, and refactor the code based on alex's review.
3) Fixed whitespace errors.
4) Fixed PermGen leak in protocol fns.

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

I screened this ticket again with Brenton Ashworth and had the following comments:

1) We need to have a performance test to verify that we have not negatively impacted performance of multimethods or protocol invocation.
2) Because there are special cases around null keys in the multimethod cache, please verify that there are existing example tests using null dispatch values in the existing test coverage.
3) In Util$RefWrapper.getObj() - why does this return this.ref at the end? It was not clear to me that the comment was correct or that this was useful in any way.
4) In Util$RefWrapper.clearRefWrapCache() - can k == null in that if check? If not, can we omit that? Also, if you explicitly create the Iterator from the entry set, you can call .remove() on it more efficiently than calling .remove() on the cache itself.
5) In core_deftype / MethodImplCache, it appears that you are modifying a now-mutable field rather than the prior version that was going to great lengths to stay immutable. It's not clear to me what the implications of this change are and that concerns me. Can it use a different collection or code to stay immutable?
6) Please update the description of this ticket to include an approach section that describes the changes we are making.

Thanks!





[CLJ-1546] Widen vec to take Iterable/IReduce Created: 02/Oct/14  Updated: 14/Nov/14

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

Type: Enhancement Priority: Critical
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File clj-1546-2.patch     Text File clj-1546-3.patch     Text File clj-1546-4.patch     Text File clj-1546.patch    
Patch: Code and Test
Approval: Incomplete

 Description   

These examples should work but does not:

Something Iterable but not IReduce:

user> (def i (eduction (map inc) (range 100)))
#'user/i
user> (instance? java.util.Collection i)
false
user> (instance? Iterable i)
true
user> (vec i)
RuntimeException Unable to convert: class clojure.core.Iteration to Object[]

Something IReduceInit but not Iterable:

user=> (vec
  (reify clojure.lang.IReduceInit
    (reduce [_ f start]
      (reduce f start (range 10)))))
RuntimeException Unable to convert: class user$reify__15 to Object[]

Proposal: Add PersistentVector.create(Iterable) and PersistentVector.create(IReduceInit) to efficiently create PVs from those. Add a number of cases to vec to support different cases.

Patch: clj-1546-4.patch

Screened by:



 Comments   
Comment by Timothy Baldridge [ 02/Oct/14 9:44 AM ]

Is there a reason the final case for (vec something) can't just be a call to (into [] coll)? It seems a bit odd to do (to-array) on anything thats not a java collection or Iterable, when we have IReduce.

Comment by Rich Hickey [ 02/Oct/14 10:02 AM ]

re: Tim - yes, this needs to support IReduce (and thereby educe) as well

Comment by Alex Miller [ 14/Oct/14 9:56 AM ]

Added new patch that handles Iterable and IReduceInit in vec. It also makes calling with a vector much faster due to the first check. into is still faster for chunked seqs (due to special InternalReduce handling of chunking).

It would be possible to move more of the variant checking into LazilyPersistentVector or PersistentVector so it could be used in more contexts. I'm not sure how much to do with that.

It would also be possible to instead lean on reduce more from the Java side if there was a Java version of reduce (as defined in mikera's branch for http://dev.clojure.org/jira/browse/CLJ-1192 at https://github.com/mikera/clojure/compare/clj-1192-vec-performance. Something like that is the only way I can see of leveraging that same InternalReduce logic that makes into faster than vec.

Comment by Alex Miller [ 13/Nov/14 4:14 PM ]

Prior comments from Stu removed from description: "Open Question: Which branch should come first, Collection or IReduceInit? Collection reaches the fast path for small collections through LazilyPersistentVector, but IReduceInit should be faster for larger things. Related: Shouldn't the item count in LazilyPersistentVector be a bounded count?"

I have attached a new patch that simplifies the impl to do it in LazilyPersistentVector instead of in vec, which was easier due to "and" not being able yet when vec is implemented to do the length check.

I have also done a considerable amount of analysis on the matrix of incoming collections and best path to follow and also collected some data on what collections are commonly passed into vec. The current patch reflects those findings. Some highlights:

  • vec is called with PersistentVector in all projects I tested. The instanceof check takes that case from typically 100s of nanos to ~5 ns. So I do think it is worth doing.
  • vec is overwhelmingly called with small collections - in most cases the incoming collection is <10 elements. In cases where the collection is not a sequence, the path of creating the Vector with an owning array is the fastest option, beating even IReduce and transient building (as that path has some checks involved).
  • PersistentList is the only IReduce likely to be encountered by vec right now and adding that branch is a significant performance boost from prior impl and vs into. If maps and sets were IReduce, they would gain this as well.
  • chunked seqs will be significantly faster with into than vec as into goes through CollReduce and can leverage many optimizations on reducing through chunks that are not available to vec.
  • seqs in general though are now faster with vec than they were due to leveraging transients.
  • eduction results support IReduce and are also faster with vec than into.
  • range is currently slower with vec, but when range is IReduce, it will probably be faster with vec

In summary, some new conventional wisdom (after this patch) on (into []) vs vec:

  • vec is faster if passed a vector, an IReduce, or an array
  • into is faster when working with seqs, but even vec is better than it used to be and may even be faster for things like range in the future
Comment by Michael Blume [ 13/Nov/14 7:24 PM ]

Latest patch won't build for me when applied to master

compile-clojure:
     [java] Exception in thread "main" java.lang.ExceptionInInitializerError
     [java] 	at clojure.lang.Compile.<clinit>(Compile.java:29)
     [java] Caused by: java.lang.NoSuchMethodError: clojure.lang.LazilyPersistentVector.create(Ljava/util/Collection;)Lclojure/lang/IPersistentVector;, compiling:(clojure/core.clj:14:23)
     [java] 	at clojure.lang.Compiler.load(Compiler.java:7206)
     [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.lang.RT.doInit(RT.java:448)
     [java] 	at clojure.lang.RT.<clinit>(RT.java:329)
     [java] 	... 1 more
     [java] Caused by: java.lang.NoSuchMethodError: clojure.lang.LazilyPersistentVector.create(Ljava/util/Collection;)Lclojure/lang/IPersistentVector;
     [java] 	at clojure.lang.LispReader$VectorReader.invoke(LispReader.java:1073)
     [java] 	at clojure.lang.LispReader.readDelimitedList(LispReader.java:1138)
     [java] 	at clojure.lang.LispReader$ListReader.invoke(LispReader.java:972)
     [java] 	at clojure.lang.LispReader.read(LispReader.java:183)
     [java] 	at clojure.lang.LispReader$WrappingReader.invoke(LispReader.java:535)
     [java] 	at clojure.lang.LispReader.readDelimitedList(LispReader.java:1138)
     [java] 	at clojure.lang.LispReader$MapReader.invoke(LispReader.java:1081)
     [java] 	at clojure.lang.LispReader.read(LispReader.java:183)
     [java] 	at clojure.lang.LispReader$MetaReader.invoke(LispReader.java:716)
     [java] 	at clojure.lang.LispReader.readDelimitedList(LispReader.java:1138)
     [java] 	at clojure.lang.LispReader$ListReader.invoke(LispReader.java:972)
     [java] 	at clojure.lang.LispReader.read(LispReader.java:183)
     [java] 	at clojure.lang.Compiler.load(Compiler.java:7190)
     [java] 	... 7 more
Comment by Alex Miller [ 13/Nov/14 7:28 PM ]

Did you clean first? I replaced that static method call there with a wider version but if you are cleaning fresh it should be fine.

Comment by Michael Blume [ 13/Nov/14 7:31 PM ]

Apologies, maven just wasn't doing a good job of tracking changes, running mvn clean fixes the build.





[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-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: 0
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-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-259] clojure.lang.Reflector.invokeMatchingMethod is not complete (rejects pontentially valid method invocations) Created: 03/Feb/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: 2
Labels: None


 Description   

There exists invoke expressions on instances, where Java is able to perform the call, yet clojure is not reflectively.
The problem is when the declaringClass of the found method is not public then the call to getAsMethodOfPublicBase uses the found method or searching for classes/interfaces that contain/define this method yet are declared publicly.

This restricts the possible search space. I suggest that if target is not null (e.g. is not a static method), the target.getClass() should be used instead as a root for getAsMethodOfPublicBase.
This fixes my issue.



 Comments   
Comment by Assembla Importer [ 24/Aug/10 3:12 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 3:12 PM ]

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

Comment by Assembla Importer [ 24/Aug/10 3:12 PM ]

hiredman said: Related association with ticket #126 was added





[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-47] GC Issue 43: Dead code in generated bytecode Created: 17/Jun/09  Updated: 26/Aug/13

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

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

Approval: Vetted

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

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

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

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


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

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

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

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

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

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

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

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





[CLJ-270] defn-created fns inherit old metadata from the Var they are assigned to Created: 13/Feb/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: Rich Hickey
Resolution: Unresolved Votes: 0
Labels: None


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

user> (def #^{:foo "bar"} x 5)
#'user/x
user> (meta #'x)
{:ns #<Namespace user>, :name x, :file "NO_SOURCE_FILE", :line 1, :foo "bar"}
user> (defn x [] 5)
#'user/x
user> (meta #'x)
{:ns #<Namespace user>, :name x, :file "NO_SOURCE_FILE", :line 1, :arglists ([])}
user> (meta x)
{:ns #<Namespace user>, :name x, :file "NO_SOURCE_FILE", :line 1, :foo "bar"}

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

I expect (meta #'x) to evaluate to the value of the final (meta x) in the above.

  • What version are you using?

Current master (commit 61202d2ff6925002400a9843e8fbd080f3bef3a5).

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

http://groups.google.com/group/clojure/browse_thread/thread/4c7151aa9c4d919c/d1b033ef5a13dd89?lnk=gst&q=off-by-one#d1b033ef5a13dd89

Prompted by discussion at

http://groups.google.com/group/clojure/browse_thread/thread/6553d48c981019eb/3c55b0bd43a5d8e9?lnk=gst&q=off-by-one#3c55b0bd43a5d8e9

  • Initial attempt at a diagnosis:

I think this is due to DefExpr's eval method binding the Var to init.eval() first and attaching the supplied metadata to the Var later – see Compiler.java lines 341-352. (Note the Var is always already in place when init.eval() is called, regardless of whether it existed prior to the evaluation of the def / defn.) Thus the init expression supplied by defn sees the old (and wrong) metadata on the Var.



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

Converted from http://www.assembla.com/spaces/clojure/tickets/270
Attachments:
dont-copy-val-metadata-onto-new-var-value-in-defn.patch - https://www.assembla.com/spaces/clojure/documents/dwK4yssayr37y_eJe5d-aX/download/dwK4yssayr37y_eJe5d-aX
0001-set-meta-on-vars-before-evaluating-their-init-see-27.patch - https://www.assembla.com/spaces/clojure/documents/arrhbiAI4r35lQeJe5cbLr/download/arrhbiAI4r35lQeJe5cbLr

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

stu said: [file:dwK4yssayr37y_eJe5d-aX]

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

stu said: The problem happens with defn, but not with def+fn, so I think the original diagnosis is incorrect.

Another stab at diagnosis: The defn macro copies metadata from a var onto its new value, so if you defn a var that already exists, the old var metadata becomes metadata on the new value. I don't know why this would be the right thing to do, and if you eliminate this behavior (see patch) all the Clojure and Contrib tests still pass.

If this is correct, please assign back to me and I will write tests. If this is wrong, please tell me what's going on here so I know how to write the tests.

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

cgrand said: Child association with ticket #363 was added

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

cgrand said: I think the original diagnosis is correct. Note that the behavior differ when def iscompiled:

user=> (def #^{:foo "bar"} x 5)
#'user/x
user=> (let [] (defn x [] 5))
#'user/x
user=> (meta x)
{:ns #<Namespace user>, :name x, :file "NO_SOURCE_PATH", :line 83, :arglists ([])}
user=> (meta #'x)
{:ns #<Namespace user>, :name x, :file "NO_SOURCE_PATH", :line 83, :arglists ([])}

See also http://groups.google.com/group/clojure/browse_thread/thread/6afd81896ca368b2#

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

cgrand said: [file:arrhbiAI4r35lQeJe5cbLr]

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

cgrand said: My patch aligns DefExpr.eval with DefExpr.emit (first set meta then eval the init value).

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

cgrand said: Forget it, my current patch is broken.

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

cgrand said: My current patch is broken because of #352, what is the rational for #352?





[CLJ-451] fn literals lack name/arglists/namespace metadata Created: 05/Oct/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: 1
Labels: None


 Description   

I would expect (meta (fn not-so-anonymous [a b c])) to include {:name not-so-anonymous :arglists ([a b c])} alongside line number information and possibly namespace/file as well, but currently it only includes :line.



 Comments   
Comment by Assembla Importer [ 05/Oct/10 12:29 AM ]

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





[CLJ-379] problem with classloader when run as windows service Created: 13/Jun/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   

I found following error when I run clojure application as MS Windows service (via procrun from Apache Daemon project). When I tried to do 'require' during run-time, I got NullPointerException. This happened as baseLoader function from RT class returned null in such environment (the value of Thread.currentThread().getContextClassLoader()). (Although my app works fine when I run my application as standalone program, not as service).
This error was fixed by explicit setting of class loader with following code:

(.setContextClassLoader (Thread/currentThread) (java.lang.ClassLoader/getSystemClassLoader))

before any call to 'require'....

May be you need to modify 'baseLoader' function, so it will check is value of Thread.currentThread().getContextClassLoader() is null or not, and if null, then return value of java.lang.ClassLoader.getSystemClassLoader() ?



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

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

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

alexott said: possible fix is attached

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

alexott said: [file:c5XWHcD4yr34HveJe5ccaP]





[CLJ-126] abstract superclass with non-public accessibility 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   

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





[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-77] GC Issue 74: Clojure compiler emits too-large classfiles (results in ClassFormatError) Created: 17/Jun/09  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   
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-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-415] smarter assert (prints locals) Created: 29/Jul/10  Updated: 03/Sep/13

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

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

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

 Description   

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

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

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


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

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

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

alexdmiller said: A simple example I tried for illustration:

user=> (let [a 1 b 2] (assert (= a b)))
#<CompilerException java.lang.AssertionError: Assert failed: (= a b)
 a : 1
 b : 2
Comment by Assembla Importer [ 24/Aug/10 5:41 PM ]

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

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

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

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

:f

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

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

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

jawolfe said: See also some comments in:

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

(defn symbols [sexp]
  "Returns just the symbols from the expression, including those
   inside literals (sets, maps, lists, vectors)."
  (distinct (filter symbol? (tree-seq coll? seq sexp))))
Comment by Andy Fingerhut [ 18/Nov/12 1:06 AM ]

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





[CLJ-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-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: 0
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-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-445] Method/Constructor resolution does not factor in widening conversion of primitive args Created: 29/Sep/10  Updated: 27/Jul/13

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

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

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

 Description   

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

Examples:

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

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

Workarounds:
Explicitly casting to the formal type.

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



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

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

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

ataggart said: [file:b6gDSUZOur36b9eJe5cbCb]

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

ataggart said: Also fixes #446.

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

The patch is causing a test failure

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

Can you take a look?

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

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

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

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

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

The patch notes:

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

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

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

Updated patch to work with latest master branch.

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

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

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

Bit by git.

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

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

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

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

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

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

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

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

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

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

Please test patch

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Created CLJ-792 for the reflector reorg.

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

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

Is CLJ-792 now a prerequisite of this ticket?

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

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

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

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

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





[CLJ-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: 1
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-1263] Allow static compilation of function invocations Created: 14/Sep/13  Updated: 07/Nov/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: 1
Labels: compiler


 Description   

This proposal is to allow metadata on functions to prevent a fully dynamic var deref to be used whenever the function is called.

When the function is invoked, JVM "invokevirtual" instruction will be used, which is faster than the current implementation (var deref + IFn cast + invokinterface) and has less restrictions (no need to predefine interfaces to match the function parameters). The JVM is generally able to compile such invokevirtual instructions into extremely efficient code - effectively as fast as pure Java.

This is intended to pave the way to better support for statically compiled, high performance code. In particular, it allow:

  • Supporting arbitrary JVM primitives (float, int, byte, char etc.) as well as just double/long.
  • Supporting typed return values e.g. "String". This could eliminate many casts and type checks.
  • Supporting typed reference arguments (e.g. String).

Suggested usage:

(defn ^:static foo ^int [^String a ^String b]
(+ (count a) (Integer/parseInt b)))

Existing code / semantics should not be affected



 Comments   
Comment by Alex Fowler [ 18/Sep/13 5:08 AM ]

Very nice! That is what would really improve experience with certain tasks. I think it will also make possible to work with primitive arrays without the conversions?

Comment by Mike Anderson [ 19/Sep/13 5:44 PM ]

Hi Alex - which aspect of "work with primitive arrays" are you referring to? This feature would certainly help with passing primitive arguments to/from functions that use primitive arrays. It would also potentially help avoid some casts of primitive array arguments themselves. I don't think it helps in any other way - perhaps a separate issue would be appropriate if there is another thing you are trying to do?

Comment by Kevin Downey [ 29/Oct/13 11:50 AM ]

this issue is confusing, because there was/is a :static feature in clojure(which seems to be disabled in the compiler at the moment) and this proposal doesn't mention the existing work at all.

I also think this proposal is begging the question, there is no discussion of other possible solutions to the performance problem (whatever that is) that this is trying to solve.

the (var.deref()(IFn)).invoke(...) is pretty fundamental to the feel of clojure, in fact the existing :static keyword seems to be disabled in the compiler exactly because it complicates those semantics. so we should have a very clear proposal (not a wishlist) if we want to change that with some very clear wins.

maybe an optimizing clojure compiler would be a better approach.

Comment by Mike Anderson [ 30/Oct/13 11:01 PM ]

Hi Kevin,

This is partly in response to this discussion on Clojure-Dev, where we discussed there are quite a lot of performance issues around the way that Clojure passes arguments currently:
https://groups.google.com/d/topic/clojure/H5P25eYKBj4/discussion

Also I believe it reinstates the original intention of "^:static": I can't find where this is/was officially documented, but Arthur's answer in this SO question suggests that this was the case:
http://stackoverflow.com/questions/7552632/what-does-static-do-in-clojure

I think the proposal is relatively clear: it's probably the minimal change required to get static/direct (i.e. not via an indirect var reference / IFn) function invocations without affecting any of the semantics of current code.

This is sufficiently important for me that it's preventing me from shifting some performance-critical code to Clojure (even with primitive type hints). e.g. here's a simple case with a small primitive function:

(defn ^long foo [^long x]
(inc x))

(c/quick-bench (dotimes [i 100] (foo i))) ;; c = criterium
=> Execution time mean : 194.334293 ns

(c/quick-bench (dotimes [i 100] (inc i)))
=> Execution time mean : 71.539048 ns

i.e. the indirect function invocation is costing us nearly 170% overhead. In Java the equivalent functions perform identically: the overhead is zero because with static function invocation the JVM JIT is able to eliminate all the function call overhead.

In the long term, I agree that a proper optimising compiler would be the best way forward (perhaps Clojure 2.0/CinC can give us this?) but in the meantime I think this is a pragmatic way to improve performance with minimal impact on existing code. Even with an optimising compiler, I think we' would need some way to specifiy the "optimised" semantics rather than the indirect var deref behaviour, and "^:static" seems like a reasonable way to do so (unless anyone has a better idea?)

Comment by Kevin Downey [ 04/Nov/13 3:58 PM ]

have you looked at the definition of int and how it uses :inline/definline to avoid the call overhead?

Comment by Mike Anderson [ 05/Nov/13 4:27 AM ]

Good point Kevin - :inline and definline seem like a good approach in many cases (although it's marked as "experimental" - does that mean we can't rely on it to work in future releases?).

This proposal is still somewhat different: the inline solutions and its variants are effectively doing macro expansion to generate code without a function call on the Clojure side. The approach in this proposal would still emit a function call in bytecode, but do so in a way that the JVM can subsequently inline and optimise much more efficiently. Both have their uses, I think?

Commented edited Nov 7 2013 by Andy Fingerhut: Regarding definline marked as experimental, it has been so marked since Clojure 1.0's release, and the plan is to keep it marked that way in the pending Clojure 1.6 release. See discussion thread on CLJ-1281. No plans to remove it that I am aware of.

Comment by Kevin Downey [ 06/Nov/13 2:06 PM ]

my point is your benchmark above is not a comparison of clojure's current deref + cast + invoke vs. invokevirtual, inc is being inlined in to a static method call there

Comment by Kevin Downey [ 06/Nov/13 2:32 PM ]

I've been noodling around this, and it is entirely possible to generate and invoke code in clojure right now without paying the extra deref() cost:

 (defn ^long fib [^long n]
   (case n
     0 0
     1 1
     (+ (fib (dec n))
        (fib  (dec (dec n))))))

can be written as

(declare TheR1798)

(definterface I1797
  (^long fib_Invoke_1 [^long n]))

(deftype R1798 []
  I1797
  (^long fib_Invoke_1
    [this1799 ^long n]
    (case n
      0 0
      1 1
      (+ (.fib_Invoke_1 this1799 (dec n))
         (.fib_Invoke_1 this1799 (dec (dec n)))))))

(def TheR1798 (new R1798))

(defn ^long fib [^long n]
  (.fib_Invoke_1 TheR1798  n)))

now the recursive calls are invokeinterfaces, and the resulting function seems to have mean execution time about 5 times smaller using criterium to bench mark

it is entirely possible to write a macro that translates one in to other, and the weird names in the above are because I have a little proof of concept that does that.

the body of the bytecode for the regular fib function first shown looks something like:

  public final java.lang.Object invokePrim(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 40
               default: 52
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          52
      34: getstatic     #33                 // Field const__1:Ljava/lang/Object;
      37: goto          94
      40: lconst_1      
      41: lload_3       
      42: lcmp          
      43: ifne          52
      46: getstatic     #37                 // Field const__3:Ljava/lang/Object;
      49: goto          94
      52: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      55: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      58: checkcast     #6                  // class clojure/lang/IFn$LO
      61: lload_1       
      62: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      65: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      70: getstatic     #57                 // Field const__5:Lclojure/lang/Var;
      73: invokevirtual #70                 // Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
      76: checkcast     #6                  // class clojure/lang/IFn$LO
      79: lload_1       
      80: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      83: invokestatic  #75                 // Method clojure/lang/Numbers.dec:(J)J
      86: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      91: invokestatic  #81                 // Method clojure/lang/Numbers.add:(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Number;
      94: areturn       
    LineNumberTable:
      line 243: 0
      line 244: 2
      line 247: 52
      line 247: 52
      line 247: 61
      line 248: 70
      line 248: 79
      line 248: 79
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      92     3 G__301   J
             0      94     0  this   Ljava/lang/Object;
             0      94     1     n   J

  public java.lang.Object invoke(java.lang.Object);
    Code:
       0: aload_0       
       1: aload_1       
       2: checkcast     #89                 // class java/lang/Number
       5: invokestatic  #93                 // Method clojure/lang/RT.longCast:(Ljava/lang/Object;)J
       8: invokeinterface #77,  3           // InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
      13: areturn       

the body of the "optimized" version looks like:

  public long fib_Invoke_1(long);
    Code:
       0: lload_1       
       1: lstore_3      
       2: lload_3       
       3: l2i           
       4: tableswitch   { // 0 to 1
                     0: 28
                     1: 38
               default: 48
          }
      28: lconst_0      
      29: lload_3       
      30: lcmp          
      31: ifne          48
      34: lconst_0      
      35: goto          80
      38: lconst_1      
      39: lload_3       
      40: lcmp          
      41: ifne          48
      44: lconst_1      
      45: goto          80
      48: aload_0       
      49: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      52: lload_1       
      53: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      56: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      61: aload_0       
      62: checkcast     #6                  // class com/thelastcitadel/kernel/I2364
      65: lload_1       
      66: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      69: invokestatic  #53                 // Method clojure/lang/Numbers.dec:(J)J
      72: invokeinterface #55,  3           // InterfaceMethod com/thelastcitadel/kernel/I2364.fib_Invoke_1:(J)J
      77: invokestatic  #59                 // Method clojure/lang/Numbers.add:(JJ)J
      80: lreturn       
    LineNumberTable:
      line 251: 0
      line 251: 2
      line 251: 48
      line 251: 48
      line 251: 52
      line 251: 61
      line 251: 65
      line 251: 65
    LocalVariableTable:
      Start  Length  Slot  Name   Signature
             2      78     3 G__2363   J
             0      80     0  this   Lcom/thelastcitadel/kernel/R2365;
             0      80     1     n   J

so the calls are not invokevirtual (due to the way clojure compiles stuff, you cannot type anything inside a record as being that record's type), but the interface is unique and only has one instance, so I think the jvm's class hierarchy analysis makes short work of that.

if I have time I may try and complete my macro and release it as a library, but given tools.analyzer.jvm someone should be able to do better than my little proof of concept very quickly.

Comment by Andy Fingerhut [ 07/Nov/13 12:48 PM ]

I don't know if my editing of Mike Anderson's Nov 5 2013 comment is notified to people watching this ticket, so adding a new comment so those interested in definline's experimental status can know to go back and re-read it.





[CLJ-457] lazy recursive definition giving incorrect results Created: 13/Oct/10  Updated: 23/Nov/13

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

Type: Defect Priority: Major
Reporter: Assembla Importer Assignee: Christophe Grand
Resolution: Unresolved Votes: 0
Labels: None

Attachments: File CLJ-457-2.diff     File clj-457-3.diff    
Patch: Code and Test

 Description   

If you define a global data var in terms of a lazy sequence referring to that same var, you can get different results depending on the chunkiness of laziness of the functions being used to build the collection.

Clojure's lazy sequences don't promise to support this, but they shouldn't return wrong answers. In the example given in http://groups.google.com/group/clojure/browse_thread/thread/1c342fad8461602d (and repeated below), Clojure should not return bad data. An error message would be good, and even an infinite loop would be more reasonable than the current behavior.

(Similar issue reported here: https://groups.google.com/d/topic/clojure/yD941fIxhyE/discussion)

(def nums (drop 2 (range)))
(def primes (cons (first nums)
             (lazy-seq (->>
               (rest nums)
               (remove
                 (fn [x]
                   (let [dividors (take-while #(<= (* % %) x)
primes)]
                     (println (str "primes = " primes))
                     (some #(= 0 (rem x %)) dividors))))))))
(take 5 primes)

It prints out:
(primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
2 3 5 7 9)


 Comments   
Comment by Assembla Importer [ 13/Oct/10 3:00 PM ]

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

Comment by Aaron Bedra [ 10/Dec/10 9:08 AM ]

Stu and Rich talked about making this an error, but it would break some existing code to do so.

Comment by Rich Hickey [ 17/Dec/10 8:03 AM ]

Is there a specific question on this?

Comment by Aaron Bedra [ 05/Jan/11 9:05 PM ]

Stu, you and I went over this but I can't remember exactly what the question was here.

Comment by Christophe Grand [ 28/Nov/12 12:24 PM ]

Tentative patch attached.
Have you an example of existing code which is broken by such a patch (as mention by Aaron Bedra)?

Comment by Rich Hickey [ 30/Nov/12 9:43 AM ]

The patch intends to do what? We have only a problem description and code. Please enumerate the plan rather than make us decipher the patch.

As a first principle, I don't want Clojure to promise that such recursively defined values are possible.

Comment by Christophe Grand [ 30/Nov/12 10:23 AM ]

The proposal here is to catch recursive seq realization (ie when computing the body of a lazy-seq attempts to access the same seq) and throw an exception.

Currently when such a case happens, the recursive access to the seq returns nil. This results in incorrect code seemingly working but producing incorrect results or even incorrect code producing correct results out of luck (see https://groups.google.com/d/topic/clojure/yD941fIxhyE/discussion for such an example).

So this patch moves around the modification to the LazySeq state (f, sv and s fields) before all potentially recursive method call (.sval in the while of .seq and .invoke in .sval) so that, upon reentrance, the state of the LazySeq is coherent and able to convey the fact the seq is already being computed.

Currently a recursive call may find f and sv cleared and concludes the computation is done and the result is in s despite s being unaffected yet.

Currently:

State f sv s
Unrealized not null null null
Realized null null anything
Being realized/recursive call from fn.invoke not null null null
Being realized/recursive call from ls.sval null null null

Note that "Being realized" states overlap with Unrealized or Realized.
(NB: "anything" includes null)

With the patch:

State f sv s
Unrealized not null null null
Realized null null anything but this
Being realized null null this
Comment by Andy Fingerhut [ 30/Nov/12 2:06 PM ]

That last comment, Christophe, goes a long way to explaining the idea to me, at least. Any chance comments with similar content could be added as part of the patch?

Comment by Christophe Grand [ 03/Dec/12 11:18 AM ]

New patch with a comment explaining the expected states.
Note: I tidied the states table up.

// Before calling user code (f.invoke() in sval and, indirectly,
// ((LazySeq)ls).sval() in seq -- and even RT.seq() in seq), ensure that 
// the LazySeq state is in one of these states:
//
// State            f          sv
// ================================
// Unrealized       not null   null
// Realized         null       null
// Being realized   null       this

CLJ-1119 is also fixed by this patch.

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

Patch clj-457-3.diff is identical to Christophe's CLJ-457-2.diff, except it has been updated to no longer conflict with the commit made for CLJ-949 on Nov 22, 2013, which basically removed the try/catch in method sval(). It passes tests, and I don't see anything wrong with it, but it would be good if Christophe could give it a look, too.





[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-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: 0
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-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: 1
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-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-1212] Silent truncation/downcasting of primitive type on reflection call to overloaded method (Math/abs) Created: 28/May/13  Updated: 05/Feb/14

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

Type: Defect Priority: Major
Reporter: Matthew Willson Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: primitives, typehints
Environment:

Clojure 1.5.1
OpenJDK Runtime Environment (IcedTea6 1.12.5) (6b27-1.12.5-0ubuntu0.12.04.1)



 Description   

I realise relying on reflection when calling these kinds of methods isn't a great idea performance-wise, but it shouldn't lead to incorrect or dangerous behaviour.

Here it seems to trigger a silent downcast of the input longs, giving a truncated integer output:

user> (defn f [a b] (Math/abs (- a b)))
Reflection warning, NO_SOURCE_PATH:1:15 - call to abs can't be resolved.
#'user/f
user> (f 1000000000000 2000000000000)
727379968
user> (class (f 1000000000000 2000000000000))
java.lang.Integer
user> (defn f [^long a ^long b] (Math/abs (- a b)))
#'user/f
user> (f 1000000000000 2000000000000)
1000000000000
user> (class (f 1000000000000 2000000000000))
java.lang.Long



 Comments   
Comment by Matthew Willson [ 28/May/13 12:50 PM ]

For an even simpler way to replicate the issue:

user> (#(Math/abs %) 1000000000000)
Reflection warning, NO_SOURCE_PATH:1:3 - call to abs can't be resolved.
727379968

Comment by Andy Fingerhut [ 28/May/13 1:36 PM ]

I was able to reproduce the behavior you see with these Java 6 JVMs on Ubuntu 12.04.2:

java version "1.6.0_27"
OpenJDK Runtime Environment (IcedTea6 1.12.5) (6b27-1.12.5-0ubuntu0.12.04.1)
OpenJDK 64-Bit Server VM (build 20.0-b12, mixed mode)

java version "1.6.0_45"
Java(TM) SE Runtime Environment (build 1.6.0_45-b06)
Java HotSpot(TM) 64-Bit Server VM (build 20.45-b01, mixed mode)

However, I tried two Java 7 JVMs, and it gave the following behavior which looks closer to what you would hope for. I do not know what is the precise difference between Java 6 and Java 7 that leads to this behavior difference, but this is some evidence that this has something to do with Java 6 vs. Java 7.

user=> (set! warn-on-reflection true)
true
user=> (defn f [a b] (Math/abs (- a b)))
Reflection warning, NO_SOURCE_PATH:1:15 - call to abs can't be resolved.
#'user/f
user=> (f 1000000000000 2000000000000)
1000000000000
user=> (class (f 1000000000000 2000000000000))
java.lang.Long

Above behavior observed with Clojure 1.5.1 on these JVMs:

Ubuntu 12.04.2 plus this JVM:
java version "1.7.0_21"
Java(TM) SE Runtime Environment (build 1.7.0_21-b11)
Java HotSpot(TM) 64-Bit Server VM (build 23.21-b01, mixed mode)

Mac OS X 10.8.3 plus this JVM:
java version "1.7.0_15"
Java(TM) SE Runtime Environment (build 1.7.0_15-b03)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)

Comment by Matthew Willson [ 29/May/13 5:17 AM ]

Ah, interesting.
Maybe it's a difference in the way the reflection API works in java 7?

Here's the bytecode generated incase anyone's curious:

public java.lang.Object invoke(java.lang.Object);
Code:
0: ldc #14; //String java.lang.Math
2: invokestatic #20; //Method java/lang/Class.forName:(Ljava/lang/String;)Ljava/lang/Class;
5: ldc #22; //String abs
7: iconst_1
8: anewarray #24; //class java/lang/Object
11: dup
12: iconst_0
13: aload_1
14: aconst_null
15: astore_1
16: aastore
17: invokestatic #30; //Method clojure/lang/Reflector.invokeStaticMethod:(Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Object;)Ljava/lang/Object;
20: areturn

Comment by Matthew Willson [ 29/May/13 5:20 AM ]

Just an idea (and maybe this is what's happening under java 7?) but given it's a static method and all available overloaded variants are presumably known at compile time, perhaps it could generate code along the lines of:

(cond
(instance? Long x) (Math/abs (long x))
(instance? Integer x) (Math/abs (int x))
;; ...
)

Comment by Andy Fingerhut [ 29/May/13 3:19 PM ]

In Reflector.java method invokeStaticMethod(Class c, String methodName, Object[] args) there is a call to getMethods() followed by a call to invokeMatchingMethod(). getMethods() returns the 4 java.lang.Math/abs methods in different orders on Java 6 and 7, causing invokeMatchingMethod() to pick a different one on the two JVMs:

java version "1.6.0_39"
Java(TM) SE Runtime Environment (build 1.6.0_39-b04)
Java HotSpot(TM) 64-Bit Server VM (build 20.14-b01, mixed mode)

user=> (pprint (seq (clojure.lang.Reflector/getMethods java.lang.Math 1 "abs" true)))
(#<Method public static int java.lang.Math.abs(int)>
#<Method public static long java.lang.Math.abs(long)>
#<Method public static float java.lang.Math.abs(float)>
#<Method public static double java.lang.Math.abs(double)>)
nil

java version "1.7.0_21"
Java(TM) SE Runtime Environment (build 1.7.0_21-b11)
Java HotSpot(TM) 64-Bit Server VM (build 23.21-b01, mixed mode)

user=> (pprint (seq (clojure.lang.Reflector/getMethods java.lang.Math 1 "abs" true)))
(#<Method public static double java.lang.Math.abs(double)>
#<Method public static float java.lang.Math.abs(float)>
#<Method public static long java.lang.Math.abs(long)>
#<Method public static int java.lang.Math.abs(int)>)
nil

That might be a sign of undesirable behavior in invokeMatchingMethod() that is too dependent upon the order of methods given to it.

As you mention, type hinting is good for avoiding the significant performance hit of reflection.





[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-792] Refactor method resolution code out of Compiler and into Reflector Created: 11/May/11  Updated: 05/Feb/14

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

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

Attachments: Text File clj-792-reorg-reflector-patch2.txt     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.





[CLJ-888] defprotocol should throw error when signatures include variable number of parameters Created: 29/Nov/11  Updated: 05/Feb/14

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

Type: Enhancement Priority: Major
Reporter: Greg Chapman Assignee: Stuart Halloway
Resolution: Unresolved Votes: 2
Labels: errormsgs, protocols

Attachments: Text File 0001-Forbid-vararg-declaration-in-defprotocol-definterfac.patch    
Patch: Code and Test
Approval: Triaged

 Description   

I tried to use & in the signature for a method in defprotocol. Apparently (see below), this is compiled so that & becomes a simple parameter name, and there is no special handling for variable number of parameters. I think the use of & in a protocol signature ought to be detected and immediately cause an exception (I also think this restriction on the signatures ought to be documented; I couldn't find it specified in the current documentation, though of course it is implied (as I later realized) by the fact that defprotocol creates a Java interface).

user=> (defprotocol Applier (app [this f & args]))
Applier
user=> (deftype A [] Applier (app [_ f & args] (prn f & args) (apply f args)))
user.A
user=> (app (A.) + 1 2)
#<core$PLUS clojure.core$PLUS@5d9d0d20> 1 2
IllegalArgumentException Don't know how to create ISeq from: java.lang.Long
clojure.lang.RT.seqFrom (RT.java:487)



 Comments   
Comment by Alex Coventry [ 21/Oct/13 4:21 PM ]

Patch with test code attached. I have it throwing a CompilerException so that it shows source code location. Not sure whether this is kosher in clojure code, but I wish more macros provided this in their error handling.

Comment by Tassilo Horn [ 22/Oct/13 6:26 AM ]

This issue has already been discussed in CLJ-1024. There I provided a patch that forbids varargs and destructuring forms at various places including defprotocol/definterface. My patch had been applied shortly before clojure 1.5 was released, but it had a bug (forbid too many uses), so it got reverted and the bug closed and declined.

I was told to bring up the issue again after 1.5 has been released.

So here is my patch again. This time it's much more relaxed and only forbids varargs in defprotocol/definterface method declarations, and in deftype/defrecord and reify method implementations.

Comment by Alex Coventry [ 22/Oct/13 7:30 AM ]

Thanks, Tassilo. If there's anywhere in the JIRA system where I could check for prior work like that for other similar issues, I'd be grateful for a pointer.

Best regards,
Alex

Comment by Tassilo Horn [ 22/Oct/13 7:39 AM ]

New version of my patch.

Now I use a CompilerException with proper file/line/column information like Alex did. I also added his test case (which passes).

Concerning your question, Alex: a search for "varargs" would have listed CLJ-1024, but probably you wouldn't have looked into it anyway, because it's a closed issue...

Comment by Tassilo Horn [ 22/Oct/13 7:44 AM ]

Alex, if you don't object could we remove your patch in favor of mine which covers a bit more cases?

Comment by Alex Coventry [ 22/Oct/13 10:57 AM ]

Yep. Just read through 1024 and the associated mailing list discussion. You should totally get the credit: Your patch is more comprehensive and you have been on this a long time. Thanks for folding in the good parts of my patch.

Best regards,
Alex

Comment by Tassilo Horn [ 22/Oct/13 12:15 PM ]

Ok, great.

It seems I don't have the permissions to delete other peoples' attachments, so could you please delete your patch yourself?

Comment by Alex Coventry [ 23/Oct/13 2:44 PM ]

Sure, Tassilo. It's done.

I think this also needs a regression test for the case hugod originally pointed out. I initially made the same mistake as you there, but amalloy pointed it out[1] before I submitted the patch, so it is a natural mistake to make and should probably be documented in the source code.

Best regards,
Alex

[1] http://logs.lazybot.org/irc.freenode.net/%23clojure/2013-10-21.txt search for 14:48:34.

Comment by Tassilo Horn [ 24/Oct/13 2:00 AM ]

Alex, I've added the regression test you suggested. Thanks for pointing that out.

Also, I added tests checking definterface method declarations, and tests checking inline method implementations made with defrecord, deftype, and reify.

However, there's a problem with the tests for deftype and reify I don't know how to fix. When I eval the macroexpand forms used in the tests in a REPL, I can see that the CompilerException is successfully thrown and printed. But it also seems to be caught somewhere in the middle, so that the macroexpand returns a form and the exception doesn't make it to the (is (thrown? ...)). Therefore, I've commented the these tests and added a big FIXME.

Comment by Tassilo Horn [ 24/Oct/13 2:28 AM ]

New version of the patch with now all tests uncommented and passing. Andy Fingerhut made me aware that for the 4 deftype and reify tests, I need eval instead of just macroexpand.

Comment by Andy Fingerhut [ 25/Oct/13 6:25 PM ]

I have not investigated the reason yet, but patch 0001-Forbid-vararg-declaration-in-defprotocol-definterfac.patch no longer applies cleanly after the latest commits to Clojure master on Oct 25 2013.

Comment by Tassilo Horn [ 28/Oct/13 2:21 AM ]

I've rebased the patch onto the current master so that it applies cleanly again.

Comment by Tassilo Horn [ 28/Oct/13 2:25 AM ]

Stu, I've assigned this issue to you because you've been assigned to CLJ-1165 which I have closed as duplicate of this issue.

One minor difference between my patch to this issue and CLJ-1165 is that here I use a CompilerException with file/line/column info whereas in CLJ-1165 I've used `ex-info`. I think the CE is more appropriate/informative, as the error is already triggered during macro expansion.





[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-1360] clojure.string/split strips trailing delimiters Created: 18/Feb/14  Updated: 18/Feb/14

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

Type: Defect Priority: Major
Reporter: Tim McCormack Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Description   

clojure.string/split and clojure.string/split-lines inherit the bizarre default behavior of java.lang.String#split(String,int) in stripping trailing consecutive delimiters:

(clojure.string/split "banana" #"an")
⇒ ["b" "" "a"]
(clojure.string/split "banana" #"na")
⇒ ["ba"]
(clojure.string/split "nanabanana" #"na")
⇒ ["" "" "ba"]

In the case of split-lines, processing a file line by line and rejoining results in truncation of trailing newlines in the file. In both cases, the behavior is surprising and cannot be inferred from the docstrings.

This behavior should either be fixed or documented.



 Comments   
Comment by Andy Fingerhut [ 18/Feb/14 10:51 AM ]

Probably documenting would be safer than changing the behavior at this point, given that some people may actually rely on the current behavior after testing, deploying, etc.

I don't currently have a suggestion for a modified doc string, but note that there are examples of this behavior and how one can use an extra "-1" limit argument at the end to get all split strings: http://clojuredocs.org/clojure_core/clojure.string/split





[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-1373] LazySeq should utilize cached hash from its underlying seq. Created: 09/Mar/14  Updated: 20/Mar/14

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

Type: Enhancement Priority: Major
Reporter: Jozef Wagner Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections, performance
Environment:

1.6.0 master SNAPSHOT


Attachments: File clj-1373.diff    
Patch: Code
Approval: Triaged

 Description   

Even if underlying seq contains a cached hash, LazySeq computes it every time.

user=> (def a (range 100000))
#'user/a
user=> (time (hash a))
"Elapsed time: 46.904273 msecs"
375952610
user=> (time (hash a)) ;; RECOMPUTE
"Elapsed time: 10.879098 msecs"
375952610
user=> (def b (seq a))
#'user/b
user=> (time (hash b))
"Elapsed time: 10.572522 msecs"
375952610
user=> (time (hash b)) ;; CACHED HASH
"Elapsed time: 0.024927 msecs"
375952610
user=> (def c (lazy-seq b))
#'user/c
user=> (time (hash c))
"Elapsed time: 12.207651 msecs"
375952610
user=> (time (hash c))
"Elapsed time: 10.995798 msecs"
375952610


 Comments   
Comment by Jozef Wagner [ 09/Mar/14 9:20 AM ]

Added patch which checks if underlying seq implements IHashEq and if yes, uses that hash instead of recomputing.





[CLJ-992] `iterate` reducer Created: 10/May/12  Updated: 09/Apr/14

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: 0001-Add-reducers-iterate.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-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: 1
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-1406] Libs are blindly added into loaded-libs even if an error occurs during loading Created: 17/Apr/14  Updated: 17/Apr/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: OHTA Shogo Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-modify-clojure.core-load-lib-so-that-it-removes-the-.patch    
Patch: Code

 Description   

Suppose you have a lib that causes some errors during loading, like the following:

(ns broken-lib)

(} ; this line will cause a reader error

And then, if you require the lib, it would be added into loaded-libs in spite of the reader error, which makes require succeed silently after that.

user=> (contains? (loaded-libs) 'broken-lib)
false
user=> (require 'broken-lib)
CompilerException java.lang.RuntimeException: Unmatched delimiter: }, compiling:(broken_lib.clj:3:3) 
user=> (contains? (loaded-libs) 'broken-lib)
true
user=> (require 'broken-lib)
nil
user=>

Cause:
The patch for CLJ-1116 made the ns macro blindly add the lib being defined into loaded-libs even if an error occurs during loading.

Approach:
Modify clojure.core/load-lib so that it removes the lib from loaded-libs on error.



 Comments   
Comment by Alex Miller [ 17/Apr/14 9:07 AM ]

This patch seems somewhat removed from the cause - is there some way to instead prevent the lib from being added to loaded-libs in the first place?

Comment by OHTA Shogo [ 17/Apr/14 9:21 AM ]

To do so, I think we need to revert CLJ-1116.





[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-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-273] def with a function value returns meta {:macro false}, but def itself doesn't have meta Created: 23/Feb/10  Updated: 18/Apr/14

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

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


 Description   

On the master (1.2) branch, if you create a def with an initial function value, {:macro false} is added to the metadata of the return value for def. However, if you look again at the metadata on the var itself, the {:macro false} is not present! This breaks the use of contrib's defalias when aliasing macros, because the new alias is marked as {:macro false}.

The code below demonstrates the issue, which was introduced in http://github.com/richhickey/clojure/commit/430dd4fa711d0008137d7a82d4b4cd27b6e2d6d1, "metadata for fns."

;; all running on 1.2, DIFF noted in comments

(defmacro foo [])
-> #'user/foo

(meta (def bar (.getRoot #'foo)))
-> {:macro false, :ns #<Namespace user>, :name bar, :file "NO_SOURCE_PATH", :line 83}
;; DIFF: where did that :macro false come from??

(def bar (.getRoot #'foo))
-> #'user/bar

(meta #'bar)
-> {:ns #<Namespace user>, :name bar, :file "NO_SOURCE_PATH", :line 84}
;; LIKE 1.1, but really weird: now the :macro false is gone again!


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

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

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

this belongs in the isssues for one of the new contrib projects (if defalias ever got moved)

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

actually, it sounds like defalias is just a place where the issue is observed but the issue is in core. The old contrib/def.clj moved to core.incubator, but defalias did not go with it.

Comment by Nicola Mometto [ 18/Apr/14 9:12 AM ]

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Var.java#L270 this is probably relevant





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

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

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

Approval: Vetted

 Description   

Simplest case:

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

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

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

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

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



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

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

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

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

Now gives exception listed.

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

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





[CLJ-995] sorted-set doesn't support IEditableCollection Created: 13/May/12  Updated: 05/Feb/14

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.





[CLJ-1399] missing field munging when recreating deftypes serialized in to byte code Created: 02/Apr/14  Updated: 02/Apr/14

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

Type: Defect Priority: Major
Reporter: Kevin Downey Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

Attachments: File clj-1399.diff    
Patch: Code

 Description   

to embed deftypes in the bytecode the compiler emits the value of each field, then emits a call to the deftypes underlying class's constructor.

to get a list of fields the compiler calls .getBasis.

the getBasis fields are the "clojure" level field names of the deftype, which the actual "jvm" level field names have been munged (replacing - with _, etc), so the compiler tries to generate code to set values on non-existent fields

https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L4579

https://www.refheap.com/70731

you can work around this by using field names that don't require munging. a solution might be just calling munge in the emission of field sets of ITypes



 Comments   
Comment by Kevin Downey [ 02/Apr/14 4:26 PM ]

reproducing case

$ rlwrap java -server -Xmx1G -Xms1G -jar /Users/hiredman/src/clojure/target/clojure-1.6.0-master-SNAPSHOT.jar
Clojure 1.6.0-master-SNAPSHOT
user=> (deftype Foo [hello-world])
user.Foo
user=> (alter-var-root #'default-data-readers assoc 'foo (fn [x] (Foo. x)))
{foo #<user$eval6$fn__7 user$eval6$fn__7@2f953efd>, inst #'clojure.instant/read-instant-date, uuid #'clojure.uuid/default-uuid-reader}
user=> #foo "1"
CompilerException java.lang.IllegalArgumentException: No matching field found: hello-world for class user.Foo, compiling:(NO_SOURCE_PATH:0:0)
user=>
Comment by Kevin Downey [ 02/Apr/14 4:39 PM ]

this patch fixes the issue on the latest master for me

Comment by Chas Emerick [ 02/Apr/14 4:57 PM ]

FWIW, this was precipitated by real experience (I think I created the refheap paste). The workaround is easy (don't use dashes in field names of deftypes you want to return from data reader functions), but I wouldn't expect anyone to guess that that wasn't already oversensitized to munging edge cases.





[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-1364] Primitive VecSeq does not implement equals or hashing methods Created: 19/Feb/14  Updated: 19/Feb/14

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

Type: Defect Priority: Major
Reporter: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: collections


 Description   

VecSeq (as produced by (seq (vector-of :int 1 2 3))) does not implements equals, hashCode, or hasheq and does not play with any other Clojure collections or sequences appropriately in this regard.

user=> (def rs (range 3))
user=> (def vs (seq (vector-of :int 0 1 2)))
user=> rs
(0 1 2)
user=> vs
(0 1 2)
user=> (.equals rs vs)
true
user=> (.equals vs rs)    ;; expect: true
false
user=> (.equiv rs vs)
true
user=> (.equiv vs rs)
true
user=> (.hashCode rs)
29824
user=> (.hashCode vs)     ;; expect to match (.hashCode rs)
2081327893
user=> (System/identityHashCode vs)  ;; show that we're just getting Object hashCode
2081327893
user=> (.hasheq rs)
29824
user=> (.hasheq vs)       ;; expect same as (.hasheq rs) but not implemented at all
IllegalArgumentException No matching field found: hasheq for class clojure.core.VecSeq  clojure.lang.Reflector.getInstanceField (Reflector.java:271)





[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-1446] (def v) with no init supplied destroys #'v metadata Created: 13/Jun/14  Updated: 13/Jun/14

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

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


 Description   

(def a) destroys #'a metadata, check this:

(def ^:mykey a 1)

(meta #'a)              ;; ok, :mykey is present

(let [v (def a)]
   [(meta v)            ;; NO :mykey present, metadata destroyed
    (identical? v #'a)  ;; true, we are talking of the same var
   ])

(meta #'a)              ;; NO :mykey present

If this is not a bug but a "feature", then we have at least two problems:

1- The def special form documentation doesn't state this behaviour at all, it needs to be clarified. With the current documentation it seems as doing a def with no init supplied will not make any side-effect at all, and this is not true for the var metadata.

2- defmulti uses this form to lookup the var and check if it already binds to a MultiFn, if that is the case then defmulti does nothing... but it really does something, defmulti will destroy the original var metadata in the (supposedly non-destructive) check. This is the involved defmulti fragment:

(let [v# (def ~mm-name)]
  (when-not (and (.hasRoot v#) (instance? clojure.lang.MultiFn (deref v#)))
   ...


 Comments   
Comment by Alex Miller [ 13/Jun/14 4:14 PM ]

I think this is mostly a dupe of CLJ-1148 but I'll leave it as it states the specific problem more precisely.

Comment by Nahuel Greco [ 13/Jun/14 7:35 PM ]

Alex Miller: It seems CLJ-1148 is an special case where this problem shows, but the patches in CLJ-1148 only fixes the issues for defonce, not generally for def, not for defmulti and not clarifies this behaviour in the def special form documentation.





[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: 0
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-1255] Support Abstract Base Classes with Java-only variant of "reify" Created: 06/Sep/13  Updated: 01/Jul/14

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

Type: Enhancement Priority: Major
Reporter: Mike Anderson Assignee: Unassigned
Resolution: Unresolved Votes: 4
Labels: interop

Approval: Triaged

 Description   

Problem:

  • Various Java APIs depend on extension of abstract base classes rather than interfaces
  • "proxy" has limitations (no access to protected fields or super)
  • "proxy" has performance overhead because of an extra layer of functions / parameter boxing etc.
  • "gen-class" is complex and is complected with compilation / bytecode generation

In summary: Clojure does not currently have a good / convenient way to extend a Java abstract base class dynamically.

The proposal is to create a variant of "reify" that allows the extension of a single abstract base class (optionally also with interfaces/protocols). Code generation would occur as if the abstract base class had been directly extended in Java (i.e. with full access to protected members and with fully type-hinted fields).

Since this is a JVM-only construct, it should not affect the portable extension methods in Clojure (deftype etc.). We propose that it is placed in an separate namespace that could become the home for other JVM-specific interop functionality, e.g. "clojure.java.interop"



 Comments   
Comment by Alex Miller [ 30/Jun/14 8:18 AM ]

From Rich: we do not want to support abstract classes in a portable construct (reify, deftype). However, this would be considered as a new Java-only construct (extend-class or reify-class). If you could modify the ticket appropriately, will move back to Triaged.





[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-1453] Most Iterator implementations do not correctly implement next failing to throw the required NoSuchElementException Created: 24/Jun/14  Updated: 07/Aug/14

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

Type: Defect Priority: Major
Reporter: Meikel Brandmeyer Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: interop

Attachments: Text File 0001-Fix-iterator-implementations-to-throw-NSEE-when-exha.patch     Text File 0001-Throw-NSEE-in-gvec-iterator.patch    
Patch: Code
Approval: Triaged

 Description   

Most implementations of Iterators for Clojure's collections do not implement the next method correctly. In case the iterator is exhausted the methods fail with some case dependent error, but not with the NoSuchElementException as required by the official Java contract for that method. This causes problems when working with Java libraries relying on that behaviour.

Issue encountered in real world code using http://pipes.tinkerpop.com.

To reproduce:

(-> [] .iterator .next)

This throws a NPE instead of NSEE.

(doto (.iterator [1 2]) .next .next .next)

This throws an ArrayIndexOutOfBoundsException instead of NSEE.

The attached patch fixes the methods by adding a check for hasNext before actually trying to provide the next element. If there is no next element the correct exception is thrown.

Up-to-date patch file: 0001-Fix-iterator-implementations-to-throw-NSEE-when-exha.patch



 Comments   
Comment by Tim McCormack [ 15/Jul/14 9:56 PM ]

To establish a baseline, this piece of code checks all the iterators I could find within Clojure 1.6.0 and makes sure they throw an appropriate exception:

iterator-checker.clj
(defn next-failure
  []
  (let [ok (atom [])]
    (doseq [[tp v]
            (sorted-map
             :vec-0 []
             :vec-n [1 2 3]
             :vec-start (subvec [1 2 3 4] 0 1)
             :vec-end (subvec [1 2 3 4] 3 4)
             :vec-ls-0 (.listIterator [])
             :vec-ls-n (.listIterator [1 2 3])
             :vec-start-ls (.listIterator (subvec [1 2 3 4] 0 1))
             :vec-end-ls (.listIterator (subvec [1 2 3 4] 3 4))
             :seq ()
             :list-n '(1 2 3)
             :set-hash-0 (hash-set)
             :set-hash-n (hash-set 1 2 3)
             :set-sor-0 (sorted-set)
             :set-sor-n (sorted-set 1 2 3)
             :map-arr-0 (array-map)
             :map-arr-n (array-map 1 2, 3 4)
             :map-hash-0 (hash-map)
             :map-hash-n (hash-map 1 2, 3 4)
             :map-sor-n (sorted-map)
             :map-sor-n (sorted-map 1 2, 3 4)
             :map-sor-ks-0 (.keys (sorted-map))
             :map-sor-ks-n (.keys (sorted-map 1 2, 3 4))
             :map-sor-vs-0 (.vals (sorted-map))
             :map-sor-vs-n (.vals (sorted-map 1 2, 3 4))
             :map-sor-rev-0 (.reverseIterator (sorted-map))
             :map-sor-rev-n (.reverseIterator (sorted-map 1 2, 3 4))
             :map-ks-0 (.keySet {})
             :map-ks-n (.keySet {1 2, 3 4})
             :map-vs-0 (.values {})
             :map-vs-n (.values {1 2, 3 4})
             :gvec-int-0 (vector-of :long)
             :gvec-int-n (vector-of :long 1 2 3))]
      (let [it (if (instance? java.util.Iterator v)
                 v
                 (.iterator v))]
        (when-not it
          (println "Null iterator:" tp))
        (try (dotimes [_ 50]
               (.next it))
             (catch java.util.NoSuchElementException nsee
               (swap! ok conj tp))
             (catch Throwable t
               (println tp "threw" (class t))))))
    (println "OK:" @ok)))

The output as of current Clojure master (201a0dd970) is:

:gvec-int-0 threw java.lang.IndexOutOfBoundsException
:gvec-int-n threw java.lang.IndexOutOfBoundsException
:map-arr-0 threw java.lang.ArrayIndexOutOfBoundsException
:map-arr-n threw java.lang.ArrayIndexOutOfBoundsException
:map-hash-0 threw java.lang.ArrayIndexOutOfBoundsException
:map-ks-0 threw java.lang.ArrayIndexOutOfBoundsException
:map-ks-n threw java.lang.ArrayIndexOutOfBoundsException
:map-sor-ks-0 threw java.util.EmptyStackException
:map-sor-ks-n threw java.util.EmptyStackException
:map-sor-n threw java.util.EmptyStackException
:map-sor-rev-0 threw java.util.EmptyStackException
:map-sor-rev-n threw java.util.EmptyStackException
:map-sor-vs-0 threw java.util.EmptyStackException
:map-sor-vs-n threw java.util.EmptyStackException
:map-vs-0 threw java.lang.ArrayIndexOutOfBoundsException
:map-vs-n threw java.lang.ArrayIndexOutOfBoundsException
:vec-0 threw java.lang.NullPointerException
:vec-end threw java.lang.ArrayIndexOutOfBoundsException
:vec-end-ls threw java.lang.IndexOutOfBoundsException
:vec-ls-0 threw java.lang.IndexOutOfBoundsException
:vec-ls-n threw java.lang.IndexOutOfBoundsException
:vec-n threw java.lang.ArrayIndexOutOfBoundsException
:vec-start threw java.lang.ArrayIndexOutOfBoundsException
:vec-start-ls threw java.lang.IndexOutOfBoundsException
OK: [:list-n :map-hash-n :seq :set-hash-0 :set-hash-n :set-sor-0 :set-sor-n]
Comment by Tim McCormack [ 15/Jul/14 9:57 PM ]

0001-Fix-iterator-implementations-to-throw-NSEE-when-exha.patch missed one thing: clojure.gvec. With the patch in place, my checker outputs the following:

:gvec-int-0 threw java.lang.IndexOutOfBoundsException
:gvec-int-n threw java.lang.IndexOutOfBoundsException
OK: [:list-n :map-arr-0 :map-arr-n :map-hash-0 :map-hash-n :map-ks-0 :map-ks-n :map-sor-ks-0 :map-sor-ks-n :map-sor-n :map-sor-rev-0 :map-sor-rev-n :map-sor-vs-0 :map-sor-vs-n :map-vs-0 :map-vs-n :seq :set-hash-0 :set-hash-n :set-sor-0 :set-sor-n :vec-0 :vec-end :vec-end-ls :vec-ls-0 :vec-ls-n :vec-n :vec-start :vec-start-ls]

That should be a quick fix.

Comment by Michał Marczyk [ 15/Jul/14 10:01 PM ]

CLJ-1416 includes a fix for gvec's iterator impls (and some other improvements to interop).

Comment by Tim McCormack [ 15/Jul/14 10:17 PM ]

Attaching a fix for the gvec iterator. Combined with the existing patch, this fixes all broken iterators that I could find.

Comment by Andy Fingerhut [ 07/Aug/14 10:25 AM ]

I believe this Clojure commit: https://github.com/clojure/clojure/commit/e7215ce82215bda33f4f0887cb88570776d558a0

introduces more implementations of the java.util.Iterator interface where next() returns null instead of throwing a NoSuchElementException





[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-1226] set! of a deftype field using field-access syntax causes ClassCastException Created: 26/Jun/13  Updated: 31/Aug/14

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

Type: Defect Priority: Major
Reporter: Nicola Mometto Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: compiler, deftype, interop

Attachments: Text File 0001-CLJ-1226-fix-set-of-instance-field-expression-that-r.patch    
Patch: Code and Test

 Description   

Clojure 1.6.0-master-SNAPSHOT

user=> (defprotocol p (f [_]))
p
user=> (deftype t [^:unsynchronized-mutable x] p (f [_] (set! (.x _) 1)))
user.t
user=> (f (t. 1))
ClassCastException user.t cannot be cast to compile__stub.user.t user.t (NO_SOURCE_FILE:1

After patch:
Clojure 1.6.0-master-SNAPSHOT
user=> (defprotocol p (f [_]))
p
user=> (deftype t [^:unsynchronized-mutable x] p (f [_] (set! (.x _) 1)))
user.t
user=> (f (t. 1))
1



 Comments   
Comment by Nicola Mometto [ 27/Jun/13 5:30 AM ]

This patch offers a better workaround for CLJ-1075, making it possible to write
(deftype foo [^:unsynchronized-mutable x] MutableX (set-x [this v] (try (set! (.x this) v)) v))





[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-1209] clojure.test does not print ex-info in error reports Created: 11/May/13  Updated: 05/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Thomas Heller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None

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.

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

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.





[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-1295] Speed up dissoc on array-maps Created: 15/Nov/13  Updated: 10/Sep/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: 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 for a matching key using indexOf(key) and saves the result in i.

If a matching key was found, the code then copies the old array to the new smaller one, but unnecessarily repeats the comparison of every key in the map to the key being removed, even though its location is already stored in i.



 Comments   
Comment by Andy Fingerhut [ 15/Nov/13 7:05 PM ]

The patch clj-1295-1.diff changes PersistentArrayMap's without() to use System.arraycopy to copy only the necessary parts from the current array to newArray, similar to PersistentHashMap's method removePair().

Benchmark 1 has strings for keys, which are relatively slow to compare to each other.

(def m1 (array-map "abcdef" 1 "abcdeg" 2 "abcdeh" 3 "abcdei" 4))
(time (dotimes [i 100000000] (dissoc m1 "abcdei")))

1.6.0-alpha2 with no changes:
"Elapsed time: 29663.443 msecs"
"Elapsed time: 29490.225 msecs"
"Elapsed time: 29600.138 msecs"
"Elapsed time: 29627.948 msecs"

1.6.0-alpha2 with patch clj-1295-1.diff:
"Elapsed time: 6362.006 msecs"
"Elapsed time: 6121.006 msecs"
"Elapsed time: 6163.377 msecs"
"Elapsed time: 6155.299 msecs"
"Elapsed time: 6395.224 msecs"

Averages about 21% of the run time before the change.

Benchmark 2 has keywords for keys, which are compared via Java ==, so as fast as comparison can get.

(def m2 (array-map :abcdef 1 :abcdeg 2 :abcdeh 3 :abcdei 4))
(time (dotimes [i 100000000] (dissoc m2 :abcdei)))

1.6.0-alpha2 with no changes:
"Elapsed time: 5033.863 msecs"
"Elapsed time: 5028.327 msecs"
"Elapsed time: 5045.019 msecs"
"Elapsed time: 5004.751 msecs"
"Elapsed time: 5039.143 msecs"

1.6.0-alpha2 with patch clj-1295-1.diff:
"Elapsed time: 2874.748 msecs"
"Elapsed time: 2862.878 msecs"
"Elapsed time: 2887.778 msecs"
"Elapsed time: 2874.196 msecs"
"Elapsed time: 2861.807 msecs"

Averages about 57% of the run time before the change.





[CLJ-1282] The quote special form should throw an exception if passed more than one form to quote Created: 23/Oct/13  Updated: 10/Sep/14

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

Type: Enhancement Priority: Major
Reporter: Howard Lewis Ship Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: errormsgs, reader

Attachments: Text File CLJ-1282-p1.patch     Text File CLJ-1282-p2.patch    
Patch: Code and Test
Approval: Triaged

 Description   

Quote currently ignores all but the first argument. In the case of being called accidentally with multiple values, it should throw an exception specifying the error.

user> (quote 1 2 3)
1

------- Original: --------

Every once in a while, you can just go down the rabbit hole.

I had an errant expression in my code:

(-> message get-message-values 'DESTINATION_MERCHANT_ID)

One would think this would work; it certainly would if the key was a keyword and not a symbol.

One would expect this to expand to:

('DESTINATION_MERCHANT_ID (get-message-values message))

however, the reader is involved, so it is as if the source were:

(-> message get-message-values (quote DESTINATION_MERCHANT_ID))

which expands to:

(quote (-> message get-message-values) DESTINATION_MERCHANT_ID))

... hilarity ensues! Because quote currently ignores extra parameters, my code gets the quoted value '(clojure.core/-> message get-message-values) rather than the expected string from the map; this shifts us from the "there's a bug in my code" to "the nature of reality is broken".

The correct expression is:

(-> message get-message-values (get 'DESTINATION_MERCHANT_ID))

This took quite a while to track down; if the

special form checked that it was passed exactly one form to quote and threw an exception otherwise, I think I would have caught this much earlier. It could even identify the expression it is quoting, which would provide a lot better understanding of where I went wrong.



 Comments   
Comment by Howard Lewis Ship [ 23/Oct/13 2:02 PM ]

Sorry, can't edit the description now to correct the formatting errors.

Comment by Howard Lewis Ship [ 24/Oct/13 6:09 PM ]

I just wanted to point out that your description is shorter, but makes it appear that such a use is unlikely and therefore unimportant; the detail of my description is to point out a reasonable situation where something explicable, but completely counterintuitive and confusing, does occur.

Comment by Alex Miller [ 24/Oct/13 8:28 PM ]

That's why I left the original in there too.

Comment by Gary Fredericks [ 09/Dec/13 7:07 AM ]

(quote) currently returns nil. Do we have an opinion about that?

Comment by Gary Fredericks [ 09/Dec/13 9:32 AM ]

Attached p1, which throws an IllegalArgumentException (wrapped in a CompilerException of course) for anything but 1 arg, and includes the number of args that were passed.

I can't think of any reason why (quote) would be useful, so I decided to throw on that too. Very easy to change of course.

Also added a test that (eval '(quote 1 2 3)) throws.

Comment by Stuart Halloway [ 31/Jan/14 12:46 PM ]

I recommend the following changes:

  • throw an ex-info that includes the offending form in its map {:form ...}
  • check only for the map data, not exception type or message, in the tests
Comment by Andy Fingerhut [ 31/Jan/14 6:31 PM ]

Patch CLJ-1282-p1.patch no longer applies cleanly after commits made to Clojure master on Jan 31 2014, probably due to the patch committed for CLJ-1318, and probably only because some lines of context changed in the test file. That would be trivial to update, but Stu's comments above suggest that more significant changes need to be made.

Comment by Gary Fredericks [ 01/Feb/14 9:19 AM ]

Throwing an ex-info is easy enough. I don't know how to avoid at least incidentally checking for the exception type, since the ExceptionInfo is wrapped in a CompilerException. I'll make a patch that keeps the class name in the test but doesn't do any checks on the cause aside from the ex-data. Let me know if I should do anything different.

Comment by Gary Fredericks [ 01/Feb/14 9:58 AM ]

Attached CLJ-1282-p2.patch which is off of the current master and addresses Stu's points.

Comment by Alex Miller [ 04/Feb/14 11:23 PM ]

Moving back to Triaged for more looks.

Comment by Nicola Mometto [ 16/Feb/14 12:12 PM ]

Currently (quote) returns nil, is it intended that this patch makes that an error or was this by accident?

Comment by Gary Fredericks [ 16/Feb/14 12:23 PM ]

I consciously chose to make (quote) an error – I made a comment about that earlier and didn't get any feedback, so I unilaterally decided to make it an error due to the fact that I couldn't think of any possible use for (quote).

It's an easy switch if somebody thinks differently.

Comment by Nicola Mometto [ 16/Feb/14 1:13 PM ]

I'm sorry I did not notice your previois comment.
I'm asking because I need to know whether I should throw on (quote) for tools.analyzer, currently it is allowed but I too think that (quote) should be an error.





[CLJ-1493] Fast keyword intern Created: 06/Aug/14  Updated: 11/Sep/14

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

Type: Enhancement Priority: Major
Reporter: dennis zhuang Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: keywords, performance
Environment:

Mac OS X 10.9.4 / 2.6 GHz Intel Core i5 / 8 GB 1600 MHz DDR3
java version "1.7.0_17"
Java(TM) SE Runtime Environment (build 1.7.0_17-b02)
Java HotSpot(TM) 64-Bit Server VM (build 23.7-b01, mixed mode)


Attachments: File fast_keyword_intern.diff    
Patch: Code
Approval: Triaged

 Description   

Keyword's intern(Symbol) method uses recursive invocation to get a valid keyword instance.I think it can be rewrite into a 'for loop'
to reduce method invocation cost.
So i developed this patch, and make some simple benchmark.Run the following command line three times after 'ant jar':

java -Xms64m -Xmx64m -cp test:clojure.jar clojure.main -e "(time (dotimes [n 10000000] (keyword (str n))))"

Before patched:

"Elapsed time: 27343.827 msecs"
"Elapsed time: 26172.653 msecs"
"Elapsed time: 25673.764 msecs"

After patched:

"Elapsed time: 24884.142 msecs"
"Elapsed time: 23933.423 msecs"
"Elapsed time: 25382.783 msecs"

It looks the patch make keyword's intern a little more fast.

The patch is attached and test.

Thanks.

P.S. I've signed the contributor agreement, and my email is killme2008@gmail.com .



 Comments   
Comment by Alex Miller [ 07/Aug/14 9:01 AM ]

Looks intriguing (and would be a nice change imo). I ran this on a json parsing benchmark I used for the keyword changes and saw ~3% improvement.

Comment by dennis zhuang [ 07/Aug/14 9:54 PM ]

Updated the patch, remove the 'k == null' clause in for loop,it's not necessary.

Comment by Andy Fingerhut [ 11/Aug/14 1:29 AM ]

Dennis, while JIRA can handle multiple patches with the same name, it can be confusing for people discussing the patches, and for some scripts I have to evaluate them. Please consider giving the patches different names (e.g. with version numbers in them), or removing older ones if they are obsolete.

Comment by dennis zhuang [ 11/Aug/14 9:19 AM ]

Hi,andy

Thank you for reminding me.I deleted the old patch.

Comment by dennis zhuang [ 11/Sep/14 10:34 AM ]

I am glad to see it is helpful.I benchmark the patch with current master branch,it's fine too.





[CLJ-1381] Improve support for extending protocols to primitive arrays Created: 13/Mar/14  Updated: 18/Sep/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: Alex Miller Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: protocols


 Description   

It is possible to extend protocols to primitive arrays but specifying the class for the type is a little tricky:

(defprotocol P (p [_]))
(extend-protocol P (Class/forName "[B") (p [_] "bytes"))
(p (byte-array 0))   ;; => "bytes"

However, things go bad if you try to do more than one of these:

(extend-protocol P 
  (Class/forName "[B") (p [_] "bytes")
  (Class/forName "[I") (p [_] "ints"))
CompilerException java.lang.UnsupportedOperationException: nth not supported on this type: Character, compiling:(NO_SOURCE_PATH:1:1)
	clojure.lang.Compiler.analyze (Compiler.java:6380)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$MapExpr.parse (Compiler.java:2879)
	clojure.lang.Compiler.analyze (Compiler.java:6369)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$InvokeExpr.parse (Compiler.java:3624)
	clojure.lang.Compiler.analyzeSeq (Compiler.java:6562)
	clojure.lang.Compiler.analyze (Compiler.java:6361)
	clojure.lang.Compiler.analyze (Compiler.java:6322)
	clojure.lang.Compiler$BodyExpr$Parser.parse (Compiler.java:5708)
	clojure.lang.Compiler$FnMethod.parse (Compiler.java:5139)
	clojure.lang.Compiler$FnExpr.parse (Compiler.java:3751)
Caused by:
UnsupportedOperationException nth not supported on this type: Character
	clojure.lang.RT.nthFrom (RT.java:857)
	clojure.lang.RT.nth (RT.java:807)
	clojure.core/emit-hinted-impl/hint--5951/fn--5953 (core_deftype.clj:758)
	clojure.core/map/fn--4207 (core.clj:2487)
	clojure.lang.LazySeq.sval (LazySeq.java:42)
	clojure.lang.LazySeq.seq (LazySeq.java:60)
	clojure.lang.RT.seq (RT.java:484)
	clojure.lang.RT.countFrom (RT.java:537)
	clojure.lang.RT.count (RT.java:530)
	clojure.lang.Cons.count (Cons.java:49)
	clojure.lang.Compiler.analyze (Compiler.java:6352)
	clojure.lang.Compiler.analyze (Compiler.java:6322)

The code in {parse-impls} is seeing the second {(Class/forName "[I")} as a function, not as a new type. One workaround for this is to only extend the protocol to one type at a time.

It would be even better (moving into enhancement area) if there was a syntax here to specify primitive array types - we already have the syntax of {bytes, ints, longs}, etc for type hints and those seem perfectly good to me.



 Comments   
Comment by Nahuel Greco [ 18/Sep/14 6:08 PM ]

It also breaks when extending only one array type:

(extend-protocol P
  String               (p [_] "string")
  (Class/forName "[B") (p [_] "ints") 
  )

;=> CompilerException java.lang.UnsupportedOperationException: nth not supported on this type ...

But changing the declaration order fixes it:

(extend-protocol P
  (Class/forName "[B") (p [_] "ints") 
  String               (p [_] "string")
  )

;=> OK




[CLJ-979] map->R returns different class when invoked from AOT code Created: 03/May/12  Updated: 22/Sep/14

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