|
Converted from http://www.assembla.com/spaces/clojure/tickets/422
Attachments:
0001-Emit-finally-exception-table-entry-for-each-try-catc.patch - https://www.assembla.com/spaces/clojure/documents/d9le_6Pvar35I1eJe5cbLr/download/d9le_6Pvar35I1eJe5cbLr
|
|
chouser@n01se.net said: [file:d9le_6Pvar35I1eJe5cbLr]: initial fix for #422
|
|
chouser@n01se.net said: It looks like javac protects the try block and each catch block individually with separate entries in the exception table, all of them pointing to the final finally block.
This patch attempts to do the same thing, and also fully reverses the "endTryCatch" commit mentioned in the ticket description. Both the examples in the ticket description work correctly, each printing just 1 2 3.
|
|
richhickey said: Thanks!
This could use some tests
|
|
chouser@n01se.net said: Joe Gallo wrote some. He said his CA is in the mail.
|
|
richhickey said: Where are they? We can try them without including them for now
|
|
chouser@n01se.net said: They were linked from the ggroup discussion. They pass.
http://gist.github.com/517871
|
|
richhickey said: Thanks, I missed that, I was looking for them in messages from Joe
|
|
stu said: Updating tickets (#404, #422, #382, #423)
|
|
chouser@n01se.net said: (In [[r:5d4022276177d562906700c428f544110a0d0f1f]]) Emit finally exception table entry for each try/catch clause. Refs #422
Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>
Branch: master
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/413
Attachments:
413-parse-args-defaults-in-enc-and-out-enc-to-UTF-8.patch - https://www.assembla.com/spaces/clojure/documents/a1Pm9AL2ar351-eJe5cbCb/download/a1Pm9AL2ar351-eJe5cbCb
|
|
bpsm said: It was tough verifying that out-enc is even honored: If out-enc is not specified, it's nil. This gets passed to the two-argument stream-to-enc. The two-argument stream-to-enc calls (stream-to-string stream nil) since nil is not equal to :bytes. This in turn calls java.io/copy with :encoding nil, which gets passed on up a chain of do-copy calls until we finally try to extract the encoding from the opts map. The accessor function (java.io/encoding opts) does the actual defaulting to UTF-8.
Maybe the implementation in shell should be more explicit?
Side note: the stream-to-enc, stream-to-bytes, stream-to-string smells like it's reinventing parts of java.io, but I've probably overlooked something. It's late where I am.
|
|
bpsm said: [file:a1Pm9AL2ar351-eJe5cbCb]
|
|
bpsm said: My analysis from last night was wrong. The defaulting of :in-enc and :out-enc actually occurs in parse-args. parse-args was still defaulting to platform default encoding despite the fact that sh clearly wants to default to UTF-8.
This likely wasn't noticed because the unit test for parse-args was not updated when it was decided to use UTF-8 for clojure.java.shell in place of the previous decision to use platform default encoding.
|
|
stu said: Thanks Marcus and Ben!
|
|
stu said: Updating tickets (#340, #399, #402, #403, #413)
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/330
Attachments:
declare-test.clj - https://www.assembla.com/spaces/clojure/documents/bZn8imvTer36QjeJe5cbCb/download/bZn8imvTer36QjeJe5cbCb
declare-preserve-meta.diff - https://www.assembla.com/spaces/clojure/documents/bwNFCIwMSr34TZeJe5cbCb/download/bwNFCIwMSr34TZeJe5cbCb
0330-def-sans-init-touches-no-meta.patch - https://www.assembla.com/spaces/clojure/documents/dRHbioAFCr34lYeJe5cbLA/download/dRHbioAFCr34lYeJe5cbLA
|
|
tomfaulhaber said: [file:bwNFCIwMSr34TZeJe5cbCb]: Patch to preserve metadata in declare
|
|
richhickey said: I'm not sure about this one. Are you evaluating code in autodoc? Otherwise, I don't see the issue for scanners/parsers
|
|
tomfaulhaber said: Yeah, my immediate case is autodoc, which loads files without knowing the user desired load order (important for things like contrib or nowadays clojure itself where there is no "root" that would pull everything in).
But the other case I was thinking of was something more like yacc where code is being generated in little snippets. Rather than building all the dependency relationships between the snippets, a program may want to just add declares to the snippet for the "external" vars that it references. Then the individual generated snippets can be emitted independently without concern for order.
At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing.
|
|
richhickey said: >At a higher level, why would declare preserve the value of a var but squash the metadata? Seems like it should be all or nothing.
Quite possibly, but then I think it might become declare == unbind-root, which doesn't help you either, does it?
|
|
tomfaulhaber said: Two issues with that, one short-term for autodoc and one more abstract:
1) With the current implementation of defmulti (even with my other patch in ticket #309), you wouldn't be able to correctly reload a file that had (declare foo) ... (defmulti foo ...). This is already causing autodoc to mis-document on of the contrib namespaces. (Because it gets loaded twice, once with a use and once with a load).
2) It means that any generation tool that wants to generate "order independent" snippets would be required to do all its own book-keeping in order to determine whether to emit a declare or not.
To me it feels like the more correct semantics would be to have declare be a no-op wrt binding. If we need something for explicitly unbinding, (def var) would be a more obvious candidate. (Right now, I think they are equivalent.)
|
|
stu said: [file:dRHbioAFCr34lYeJe5cbLA]
|
|
stu said: Third patch takes a different approach, working at level of def instead of declare. Def forms without an init-expr set no metadata.
Questions:
(1) Tom, does this solve the problems you are seeing?
(2) Rich, can DefExpr's .meta be typed as MapExpr, instead of Expr?
(3) A further wrinkle would be to def-sans-init set metadata if no metadata is present yet. Too cute?
|
|
stu said: Updating tickets (#330, #357, #358, #365)
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/321
Attachments:
added-metadata.patch - https://www.assembla.com/spaces/clojure/documents/adfZhGuYCr342jeJe5cbCb/download/adfZhGuYCr342jeJe5cbCb
meta-added-after-docstring.patch - https://www.assembla.com/spaces/clojure/documents/bKqdmMu5yr37KAeJe5cbLA/download/bKqdmMu5yr37KAeJe5cbLA
added-metadata-for-public-doced-vars.patch - https://www.assembla.com/spaces/clojure/documents/d-5xbmu50r363qeJe5cbLA/download/d-5xbmu50r363qeJe5cbLA
|
|
stu said: # added from 1.0 to 1.1
clojure.core/assert
clojure.core/->>
clojure.core/assoc!
clojure.core/boolean-array
clojure.core/booleans
clojure.core/bound-fn
clojure.core/bound-fn*
clojure.core/byte-array
clojure.core/bytes
clojure.core/char-array
clojure.core/char?
clojure.core/chars
clojure.core/chunk
clojure.core/chunk-append
clojure.core/chunk-buffer
clojure.core/chunk-cons
clojure.core/chunk-first
clojure.core/chunk-next
clojure.core/chunk-rest
clojure.core/chunked-seq?
clojure.core/conj!
clojure.core/deliver
clojure.core/disj!
clojure.core/dissoc!
clojure.core/future-cancel
clojure.core/future-cancelled?
clojure.core/future-done?
clojure.core/future?
clojure.core/get-thread-bindings
clojure.core/juxt
clojure.core/persistent!
clojure.core/pop!
clojure.core/pop-thread-bindings
clojure.core/promise
clojure.core/push-thread-bindings
clojure.core/ref-history-count
clojure.core/ref-max-history
clojure.core/ref-min-history
clojure.core/short-array
clojure.core/shorts
clojure.core/sorted-set-by
clojure.core/take-last
clojure.core/transient
clojure.core/with-bindings
clojure.core/with-bindings*
clojure.core/with-loading-context
clojure.stacktrace/e
clojure.stacktrace/print-cause-trace
clojure.stacktrace/print-stack-trace
clojure.stacktrace/print-throwable
clojure.stacktrace/print-trace-element
clojure.stacktrace/root-cause
clojure.test/initial-report-counters
clojure.test/load-tests
clojure.test/report-counters
clojure.test/stack-trace-depth
clojure.test/test-out
clojure.test/testing-contexts
clojure.test/testing-vars
clojure.test/are
clojure.test/assert-any
clojure.test/assert-expr
clojure.test/assert-predicate
clojure.test/compose-fixtures
clojure.test/deftest
clojure.test/deftest-
clojure.test/file-position
clojure.test/function?
clojure.test/get-possibly-unbound-var
clojure.test/inc-report-counter
clojure.test/is
clojure.test/join-fixtures
clojure.test/report
clojure.test/run-all-tests
clojure.test/run-tests
clojure.test/set-test
clojure.test/successful?
clojure.test/test-all-vars
clojure.test/test-ns
clojure.test/test-var
clojure.test/testing
clojure.test/testing-contexts-str
clojure.test/testing-vars-str
clojure.test/try-expr
clojure.test/use-fixtures
clojure.test/with-test
clojure.test/with-test-out
clojure.walk/keywordize-keys
clojure.walk/macroexpand-all
clojure.walk/postwalk
clojure.walk/postwalk-demo
clojure.walk/postwalk-replace
clojure.walk/prewalk
clojure.walk/prewalk-demo
clojure.walk/prewalk-replace
clojure.walk/stringify-keys
clojure.walk/walk
|
|
stu said: ; Added from 1.1 to 1.2
clojure.core/-cache-protocol-fn
clojure.core/-reset-methods
clojure.core/EMPTY-NODE
clojure.core/agent-error
clojure.core/bound?
clojure.core/case
clojure.core/definterface
clojure.core/defprotocol
clojure.core/defrecord
clojure.core/deftype
clojure.core/denominator
clojure.core/error-handler
clojure.core/error-mode
clojure.core/extend
clojure.core/extend-protocol
clojure.core/extend-type
clojure.core/extenders
clojure.core/extends?
clojure.core/find-protocol-impl
clojure.core/find-protocol-method
clojure.core/flatten
clojure.core/frequencies
clojure.core/group-by
clojure.core/hash-combine
clojure.core/munge
clojure.core/namespace-munge
clojure.core/numerator
clojure.core/object-array
clojure.core/partition-all
clojure.core/partition-by
clojure.core/rand-nth
clojure.core/reductions
clojure.core/reify
clojure.core/remove-all-methods
clojure.core/restart-agent
clojure.core/satisfies?
clojure.core/seq-contains?
clojure.core/set-error-handler!
clojure.core/set-error-mode!
clojure.core/shuffle
clojure.core/thread-bound?
clojure.core/vector-of
|
|
stu said: [file:adfZhGuYCr342jeJe5cbCb]
|
|
stu said: [file:bKqdmMu5yr37KAeJe5cbLA]
|
|
stu said: April 29 patch puts metadata after docstring, and replaces previous patches.
|
|
stu said: [file:d-5xbmu50r363qeJe5cbLA]
|
|
stu said: Third patch is the one. Adds metadata after docstring, public doc-ed vars only.
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/318
Attachments:
add-annotation-support-to-gen-class.patch - https://www.assembla.com/spaces/clojure/documents/bK_ToyuHSr36E9eJe5cbLA/download/bK_ToyuHSr36E9eJe5cbLA
add-annotations-to-gen-class-v2.patch - https://www.assembla.com/spaces/clojure/documents/b1J4pcwjqr35cDeJe5cbCb/download/b1J4pcwjqr35cDeJe5cbCb
|
|
dsg said: Here is my implementation of annotations for gen-class. At present, it only adds annotations to the class, declared (not inherited) methods, and parameters in declared methods. The syntax mimics the syntax from deftype and is viewable in test/clojure/test_clojure/genclass/examples.clj.
|
|
dsg said: [file:bK_ToyuHSr36E9eJe5cbLA]: Patch that adds annotation support to gen-class.
|
|
stu said: Hi Daniel,
I found out the hard way that using a decent sample of annotation classes in the tests will break the official build (which has to run on Java 1.5). I finessed this issue by splitting the tests into separate Java 1.5 and Java 1.6 files, you can see this in clojure/test_clojure/annotations.clj
This patch will need something similar. Do you have time to get to it in the next few days?
Stu
|
|
dsg said: Hello, Stu,
Yes, I noticed your other updates to the other bug, but haven't taken the time to look at it in detail, yet. I will make some time to fix this in the next day or two.
Sincerely,
Daniel Solano G��mez
|
|
dsg said: Unfortunately, since gen-class depends on AOT compilation, fixing the Java 5 issue is not as simple for gen-class as it is for deftype. Most solutions will require changes to the build script. Here are my proposed solutions:
Option 1: Disable tests for Java 5
Simply don't try to compile the test class and skip the annotation tests if the VM is version 5. Given that Java 5 from Oracle has reached its end of life, this option has the appeal of simplicity.
Option 2: The deftype approach
Like deftype, create a simplified set of tests for Java 5. The build script will have to conditionally compile either a Java 5 test class or a Java 6+ test class.
Option 3: Custom annotations
Write a custom set of annotations (probably in Java). This way the tests will not be dependent on which JVM version is being used. This would keep the test code itself simple, and could be reused by the deftype tests.
Option 4: Restricted annotations
Restrict the annotations used to those available in Java 5. Unfortunately, there are not many, and I don't think that any of them allow features like nested annotations. I could be mistaken, though.
I appreciate any opinion on what is the preferred way to handle this problem.
|
|
stu said: The official build of Clojure is still Java 5, so let's do Option 4 for now. If we ever get burned by it, we'll create a ticket for options 3.
|
|
dsg said: OK, no problem. I will try to see if it's possible to do some things like nesting with the Java 5 annotation set. The annotations may not be semantically sound, but they may work nonetheless. Off-hand, it seems like many restrictions are enforced only at compile time.
I will submit an updated patch later this afternoon.
|
|
dsg said: [file:b1J4pcwjqr35cDeJe5cbCb]: New patch, Java 5 compatible.
|
|
dsg said: Updated patch, tested with both Sun JDK 1.5.0.22 and 1.6.0.20 on Linux.
|
|
importer said: (In [[r:8b1ea574a61b105b9cbe9ffa3b05785cec0b3bc0]]) Add annotations support to gen-class, plus tests. See #318.
Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>
Branch: master
|
|
stu said: Updating tickets (#310, #318, #341)
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/310
Attachments:
clojure-promote-repl.diff - https://www.assembla.com/spaces/clojure/documents/c6elQ8u8ur34WNeJe5cbLA/download/c6elQ8u8ur34WNeJe5cbLA
clojure-contrib-promote-repl.diff - https://www.assembla.com/spaces/clojure/documents/c6qz7Ou8ur34WNeJe5cbLA/download/c6qz7Ou8ur34WNeJe5cbLA
0310-promote-repl-clojure-with-tweaks.patch - https://www.assembla.com/spaces/clojure/documents/cEvRP-wc8r36MfeJe5cbCb/download/cEvRP-wc8r36MfeJe5cbCb
0310-clojure-repl-with-fn-naming.patch - https://www.assembla.com/spaces/clojure/documents/d6k246wFSr35tLeJe5cbLr/download/d6k246wFSr35tLeJe5cbLr
|
|
chouser@n01se.net said: Not interested in "show", perhaps with a better name? clojure.java/classinfo or something?
For tickets like this do you actually want a patch against clojure, or is this more of a placeholder for eventual work by someone else?
|
|
scgilardi said: - "show" is the repl-util I use most often. I think it should be included.
- Abstractly I think these are more development tools than things particularly about a repl. Perhaps clojure.dev would be a better name than clojure.repl.
|
|
stu said: updated description, hopefully now dev-able by anybody
|
|
stu said: pulling javadoc for now, due to the number of dependencies. May re-open it as a separate ticket.
|
|
tvachon said: [file:c6elQ8u8ur34WNeJe5cbLA]: Clojure core patch
|
|
tvachon said: [file:c6qz7Ou8ur34WNeJe5cbLA]: Clojure contrib patch
|
|
tvachon said: A couple items I'm not sure I got right:
- promoting macros necessitated promoting the functions they used directly, but this could be avoided with * functions, I believe
- some public contrib functions are now repeated verbatim as private functions in clojure.repl. I left comments in clojure.contrib, which may not be appropriate.
Let me know - happy to update the patches.
|
|
stu said: Wow, print-dir sure sucks in a lot of stuff. I think we can avoid it. Can you update the clojure patch to:
- leave out throwable?, throwable, and throw-if, and get-ns
- have print-dir call find-ns directly, do an if-let existence test, and throw an exception if the ns is not found
Thanks!
|
|
tvachon said: Indeed! Patches are updated, and I think they're much nicer.
I've also restored print-dir in contrib and added dir* in clojure.repl, which has slightly different semantics (in particular, it expects a symbol input, not a Namespace). We don't use print-dir anywhere in clojure-contrib now, but in case anyone else does this should minimize issues.
|
|
stu said: Thanks Travis,
Thanks to the new "last var in wins" feature in Clojure, there is no need to remove these fns from clojure-contrib. They can stay there, and not break people who already use them that way. So we will apply only Clojure-side patch.
|
|
stu said: [file:cEvRP-wc8r36MfeJe5cbCb]
|
|
stu said: My May 5 patch includes Travis' original patch for the Clojure side, and adds a second commit that puts the (use 'clojure.repl) in the right place in main.clj, and removes the dependency on the now-defunct "seq-contains?". The two patch approach preserves provenance but means that there is an intermediate commit where the build is broken.
Should we move towards a common convention for relating the names of macros and their driver fns? This patch has dir/dir* but source/get-source.
|
|
richhickey said: Whatever the convention is, I'm wary of blah*. In Clojure the * version is often a primitive thing. Something* is also a nice (as yet unused) variant for other meanings. Double leading underscore is pretty ugly and obviously internal to most people
|
|
richhickey said: blah-impl is fine too
|
|
tvachon said: I like the double leading underscore. Stuart - would you like me to update the original patch? I'd be happy to incorporate your changes as well if you think that's appropriate.
It's a minor matter, but I also noticed that I'm using my weatherbill.com email address, which maybe isn't ideal, as this work wasn't done in that scope. That leads to a second question, probably best taken out of this but, about corporate CLAs, but I'll bring that up in a different forum.
Thanks!
|
|
tvachon said: er - s/taken out of this but/taken out of this ticket/
|
|
stu said: In this case (and perhaps in many cases) the driver fns might actually need to be called, so I don't like the __. blah-impl is fine, if you would like to make that change please do.
|
|
richhickey said: If we are intending to have people call the fns, like dir* and get-source, then what is here is not ok.
If the macros are just macros for quoting purposes, I think we should establish a macro/macro-fn naming convention. Also, the dir* fn probably shouldn't print, i.e. the functions, if they are to have utility as functions, should be (real) functions, i.e. no side effects, return data structures, in which case the division of labor between dir and dir* (to be dir-fn) needs to change.
|
|
stu said: [file:d6k246wFSr35tLeJe5cbLr]
|
|
stu said: may 7 patch subsumes the others and is ready for review
|
|
stu said: Updating tickets (#310, #318, #341)
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/289
Attachments:
internal-reduce.patch - https://www.assembla.com/spaces/clojure/documents/a45y7yqser379teJe5aVNr/download/a45y7yqser379teJe5aVNr
|
|
stu said: [file:a45y7yqser379teJe5aVNr]: internal-reduce
|
|
liwp said: Stuart,
Great stuff!
You could move the (alength arr) call in arr-impl from the loop form to the let-binding to avoid it getting called on each iteration.
– Lauri
|
|
khinsen said: This looks like a very good idea!
Question: would it be preferable to pass the original coll to internal-reduce, rather than (seq coll)?
Advantages:
- no need to create a seq before doing a reduce
- more potential for optimization if the coll can implement reduce faster directly and the associated seq does not permit to retrieve the coll.
Disadvantage:
- reduce needs to be implemented for seqs anyway, so the number of implementations would increase.
Konrad
|
|
stu said: Lauri,
I tried lifting the alength calls and the code runs slightly slower (in my unscientific tests on a single laptop. Counterintuitive, huh?
Stu
|
|
stu said: Konrad,
The chunking support is on seqs, and it is fast. Also the seq-before-reduce localizes the nil/empty check in reduce. I think Rich had another reason too but I can't remember what it was. 
Stu
|
|
stu said: Updating tickets (#289, #298)
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/281
Attachments:
serial.diff - https://www.assembla.com/spaces/clojure/documents/aTM6uyl7Kr34JteJe5avMc/download/aTM6uyl7Kr34JteJe5avMc
serial2.diff - https://www.assembla.com/spaces/clojure/documents/dwWwVQmG0r37KxeJe5aVNr/download/dwWwVQmG0r37KxeJe5aVNr
serialization.diff - https://www.assembla.com/spaces/clojure/documents/bJZ074vWWr34fReJe5cbLA/download/bJZ074vWWr34fReJe5cbLA
281-tweaked-to-apply-to-current-master.patch - https://www.assembla.com/spaces/clojure/documents/dZLUVGv2yr35hJeJe5cbCb/download/dZLUVGv2yr35hJeJe5cbCb
|
|
bhurt said: [file:dwWwVQmG0r37KxeJe5aVNr]: New diff
|
|
bhurt said: Just added new file serial2.diff, which is a new diff which adds a few more classes to what is serialized:
Cons
PersistentHashMap$ArrayNode
PersistentHashMap$BitmapIndexedNode
PersistentHashMap$HashCollisionNode
Note that serial2.diff implements all the changes serial.diff implements, plus these new classes. So it's a complete replacement.
|
|
dsg said: I am not sure that simply tagging Clojure's data structures as serializable is the best way to handle this issue. I have not taken too much time to think about this, but here are some of the issues I do not think have been addressed in this patch:
1. Given the shared structure of most of Clojure's data types, I think care should be taken to only serialize the logical snapshot of a structure, not the entire history.
2. I am not sure that these patches even work. For example, PersistentVector$Node is marked serializable, but part of its state is an AtomicReference<Thread>. It doesn't make any sense to serialize a reference to a thread. In fact, I don't think Thread is serializable at all. If the reference is set, this will throw a NotSerializableException, I believe.
In the end, I think the best (and perhaps only) way to handle serialization for Clojure's data structures is to use Externalizable.
|
|
bhurt said: 1. I thought that the clojure data structures were just immutable- and that thus they didn't keep their history. If I'm wrong, this is a significant problem with clojure, as keeping the history of the data structure would interfere with garbage collecting.
2. The only times the edit reference is set to not null is when the vector is a transient vector- and I'm explicitly not allowing the serialization of transient vectors. But I think you could still hit it if some other thread has converted the vector to being a transient while the current thread was trying to serialize the non-transient version of the vector. Which at least explains why I haven't hit this bug yet. In any case, no serializing the edit field is probably a good idea in general.
I'll write a new patch to address these problems probably over the weekend.
|
|
cemerick said: Clojure data structures only retain the portions of their "predecessors" as required by the "current version" of the data structure. So, there should never be any dead nodes, etc. in the "entire history" of a data structure.
The AtomicReference<Thread> field is never set to null if I'm reading the code right – rather, it is set to an AtomicReference containing null (see the NOEDIT static field). I think the most straightforward way to make this serialization-friendly is to mark the edit field (all all similar fields in other data structures) transient, but default the field to the NOEDIT object so that deserialized instances have a sane value there.
|
|
bhurt said: The edit member variable (which holds the atomic reference cell) needs to be set up correctly- if I'm reading the code correctly, it's shared among all copies of the data structure. So when we deserialize a persistent vector, we need to share the reference cell among all the nodes.
Actually, the issue of sharing is one I haven't thought about much. Consider the case where you serialize multiple versions of the same vector that share a lot of nodes. Do the deserialized vectors need to share the nodes as well?
|
|
cemerick said: ObjectOutputStream and ObjectInputStream are smart about using references to previously-written objects instead of duplicating serializations. So if you write three different large vectors that share a lot of internal structure to the same OOS, that shared structure will be written only once.
As for the edit field, I think you can just default it to NOEDIT. To be clear, that field should never be serialized to begin with, so whatever the default value is in the code will be the value used by deserialized instances.
|
|
richhickey said: This patch does not apply. Please format your patch with git format-patch as described here:
http://clojure.org/patches
Thanks
|
|
cemerick said: Taking this on, hoping to get it in under the wire for clojure 1.2 RC.
|
|
cemerick said: [file:bJZ074vWWr34fReJe5cbLA]
|
|
cemerick said: Patch attached. It further requires the fix in the patch attached to #331.
|
|
stu said: [file:dZLUVGv2yr35hJeJe5cbCb]
|
|
stu said: (My attachment simply updates Chas' patch to work with current master.)
Looks good, tests pass. One question: What is the use case for serializing a fn? Deserializing a fn will not work across VMs, as the fn internally refers to anonymous classes that no longer exist. Without knowing the use case I would be tempted to pull this out because of the confusion it could cause.
The only other minor interesting thing to note is that serializing a namespace just serializes the namespace, not its contents. This makes sense once you think about it, and is necessary for serializing structs and records, but might be worth a doc note.
|
|
cemerick said: I've mentioned in #clojure a couple of times (and it's worth adding here "for the record") that all of the support for Java serialization should be and now is only present with the use case in mind of short term storage, not long term archiving – the class hierarchy implementing all of the standard data structures will change, and nothing short of a print-dup-esque mechanism (which could be layered on top of Serializable for interop purposes) will ensure longevity of serialized Clojure objects. More concretely, think sessions, RMI, etc.
Anyway: functions have been serializable for a while now, so I added tests to that end. As you said, the serializability of functions has a much narrower useful lifespan at the moment. I personally don't know of a use case for same-VM serialization of functions; I wonder if Rich does, or if that was just as far as he got in enabling deeper support for fn serialization. If the latter, then yes, we should probably pull it out; I can squash that change and the metadata tweak you added if you like.
I don't know that any documentation of how namespaces are serialized will be anything other than confusing – it's purely an implementation detail that essentially no one will (or should) ever care about. And, FWIW, the automatic interning of namespaces is essentially implied by the docs on create-ns anyway.
|
|
cemerick said: From an email exchange between myself, S. Halloway, and Rich addressing the serializability of functions:
Rich: If the code is AOT compiled and deployed to both VMs, they will have the same classnames for the fns.
S. Halloway: But serialization doesn't know this, and will happily serialize runtime-created fns. If the mangled names happen to exist in the new VM, you would end up calling different fns...
Rich: As you say, for consenting adults.
Chas: I forgot about the (pleasant) AOT caveat. Yessir, that's useful.
Is there any way to tell where a class originated from – a classfile vs. runtime codegen? If so, I could put the necessary check-and-throw in so that non-AOT'd functions would fail to serialize.
Rich: This really isn't different from the versioning problem inherent with the data structures as well, it's just that the unit of change is a compilation, not a release. You don't really protect anything with the AOT check since AOT'ed in two different compilations is potentially different, but might have same name.
I don't think we should spend any time trying to protect anyone on this. We can just document the behavior and limitations. Serializing closures is fancy stuff for experts in any case.
|
|
stu said: Updating tickets (#281, #331)
|
|
bhurt said: I just wanted to say Thank You to Chas for picking this up.
|
|
cemerick said: @Brian: No problem, thanks for getting it started.
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/269
Attachments:
0001-gvec-implement-Iterable-j.u.Collection-and-j.u.Li.patch - https://www.assembla.com/spaces/clojure/documents/b_GjzsgfOr36H0eJe5aVNr/download/b_GjzsgfOr36H0eJe5aVNr
|
|
richhickey said: Parent association with ticket #264 was added
|
|
chouser@n01se.net said: [file:b_GjzsgfOr36H0eJe5aVNr]: gvec: implement Iterable, j.u.Collection, and j.u.List
|
|
chouser@n01se.net said: Nothing dramatic in the attached patch. A couple things I was unsure about:
1. There are a couple places where walking through part or all of the vector is done by incrementing an index and calling .nth each time. Is it worth while to instead use a chunked seq of the vector?
2. In a couple other places I use the seq library: some and every? Neither of these use chunks. Is this acceptable? If not, would it be acceptable if those fns used chunks?
3. Why oh why are Iterator and ListIterator such terrible APIs? Ok, I'm not really unsure about that one...
--Chouser
|
|
chouser@n01se.net said: Oh, one more:
4. I used subvec on a gvec to implement subList. It seems to work fine since APersistentVector.SubVector just requires the underlying object to be an IPersistentVector, which of course gvecs are. This is ok?
|
|
richhickey said: This all seems ok as a first cut. I wouldn't do anything with chunks in here right now. subvec seems ok. some/every? we might reconsider at some point due to bootstrapping. But that is an implementation detail
|
|
chouser@n01se.net said: (In [[r:df17f32858eb0b700d21a2a2364a54596f139e89]]) gvec: implement Iterable, j.u.Collection, and j.u.List Fixes #268 and #269
Signed-off-by: Rich Hickey <richhickey@gmail.com>
Branch: master
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/266
Attachments:
fix266.patch - https://www.assembla.com/spaces/clojure/documents/aUPsnCrXyr36i1eJe5avMc/download/aUPsnCrXyr36i1eJe5avMc
add_comparable_support_to_vec.patch - https://www.assembla.com/spaces/clojure/documents/bt20JetxCr36GpeJe5d-aX/download/bt20JetxCr36GpeJe5d-aX
add_comparable_support_to_vec-revised.patch - https://www.assembla.com/spaces/clojure/documents/c2n-HUtXWr34oReJe5dVir/download/c2n-HUtXWr34oReJe5dVir
eliminate-reflection-in-266.patch - https://www.assembla.com/spaces/clojure/documents/d_AO6it8Sr37m4eJe5dVir/download/d_AO6it8Sr37m4eJe5dVir
|
|
richhickey said: Parent association with ticket #264 was added
|
|
dsg said: This is my first attempt at submitting a real patch to Clojure, so I appreciate any feedback. This patch actually does a few different things, specifically:
1. Add java.lang.Comparable interface for Vec.
2. Modify vector-of to be a little more user-friendly. The original only creates empty Vecs. My patch allows vector-of to take arguments similar to vector and vec. Importantly, it also takes a primitive array as an argument. The implementation may not be the most efficient, but it seems to work.
3. I added a bunch of tests to ensure that vector-of mirrors the behaviour of standard vectors. In doing so, I ran into a few bugs that led me to implement the Associative interface for Vec and IPersistentCollection to VecSeq.
It is still probably a good idea to add more tests.
|
|
dsg said: [file:aUPsnCrXyr36i1eJe5avMc]: Patch to fix #266
|
|
stu said: Hi Daniel,
Thanks for jumping on this. I am reviewing the patch, and have hit one issue so far. It's a subtle one:
(let [foo (into (vector-of :int) [1 2 3 4])]
(println "count: "(count (seq (next foo))))
(println ".count: " (.count (seq (next foo)))))
Your count isn't taking account of the offset in the VecSeq. This is hidden by a weakness in Clojure's count fn: it doesn't know when part of a seq is counted to take advantage of that, so you can't even see the issue by calling count!
This makes me think we need to test every Java interface method directly. 
|
|
dsg said: Parent association with ticket #297 was added
|
|
dsg said: [file:bt20JetxCr36GpeJe5d-aX]: Adds Comparable support to Vec, with tests.
|
|
dsg said: New patch for Comparable support. I'll move the other fixes/features to other bugs. This new patch (dated 21 Apr), also includes tests specifically for Comparable.
|
|
stu said: Hi Daniel,
This looks great. One tweak: it is not idiomatic (in Clojure at least) to do the type check. All the other Comparable types in Clojure simply throw a ClassCastException when you pass a bad arg to .compareTo.
If you convert that check to a Clojure (cast ...) and update the tests, I think we're good to go.
(I can also make this change, but I want to let the whole thing go in under your name.)
Cheers,
Stu
|
|
dsg said: Hello, Stu,
Thanks for the feedback. I'll make your suggested changes and open those other tickets tomorrow.
Sincerely,
Daniel Solano G��mez
|
|
dsg said: [file:c2n-HUtXWr34oReJe5dVir]: Revised patch to add Comparable support to Vec.
|
|
dsg said: I have submitted a new patch 'add_comparable_support_to_vec-revised.patch', which includes Stu���s recommendations and adds a couple of extra tests.
|
|
stu said: [file:d_AO6it8Sr37m4eJe5dVir]
|
|
stu said: recommend the apr 23 patch (which solves the problem) plus the apr 24 patch (which removes reflection from the api)
|
|
importer said: (In [[r:a3d1d494e9e574599c50dd83749183c66f653192]]) Add Comparable support to Vec, with tests. Fixes #266.
Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>
Branch: master
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/232
Attachments:
locals.clj - https://www.assembla.com/spaces/clojure/documents/d6rA1a9ASr3RHaeJe5afGb/download/d6rA1a9ASr3RHaeJe5afGb
|
|
technomancy said: [file:d6rA1a9ASr3RHaeJe5afGb]: repro case
|
|
chouser@n01se.net said:
((let [x :foo]
(#^{:once true} fn* []
(try (#(throw (Exception.)))
(catch Exception e
(println "x:" x))))))
</code></pre>
The same problem is visible when a finally clause uses a closed-over:
<pre><code> ((let [x :foo]
(#^{:once true} fn* []
(try (#(throw (Exception.)))
(finally
(println "x:" x))))))
Both of these print x: nil when they should print x: :foo
I think the problem is that, because the last expr in a try block produces the value that will be returned, it is compiled in a "return" context (a.k.a. tail position), therefore the locals are cleared before calling the final function.
But if that final call throws an exception, you end up in the catch clause with your locals cleared.
emitClearLocals protects against this problem by using localsUsedInCatchFinally, but emitClearCloses (used only when :once is true) does not.
The solution is so very far beyond me. I guess the simplest might be to skip clearing of closed-overs that are used in catch/finally clauses, but I don't think those are currently tracked. It looks like closeOver could be made to track these as well, but I'm lost as to the relationships between the various instances in that code.
|
|
danlarkin said: Kevin Downey and I just stumbled on this problem again. Here's our smallest repro case:
(defn throwsomething [] (throw (Exception.)))
(defn foo [bar]
@(delay (try
(throwsomething)
(catch Exception e
(nil? bar)))))
</code></pre>
We came up with two workarounds, one is to wrap everything inside the delay in a let capturing the "bar" scope, like this:
<pre><code>
(defn foo [bar]
@(delay (let [bar bar]
(try
(throwsomething)
(catch Exception e
(nil? bar))))))
</code></pre>
and the other is to not have the exception-throwing call in the tail position, like this:
<pre><code>
(defn foo [bar]
@(delay (try
(let [t (throwsomething)]
t)
(catch Exception e
(nil? bar)))))
|
|
richhickey said: (In [[r:93fecbd825c26e2570f8449cd64d0df0cc520c1d]]) fold closes clearing into the path system, fixes #232
Branch: master
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/119
|
|
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)
|
|
troussan said: The reflection warning in the example code is caused by start and length parameters. If you replace the line 98 with:
(.append buffer ch (int start) (int length)) ;;;; line 98
the call to append is resolved.
|
|
richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)
|
|
richhickey said: Updating tickets (#94, #96, #104, #119, #163)
|
|
stu said: To prevent the reflection warning, use primitive coercions as needed:
(.append *buffer* ch start length)
I have update the docs on clojure.org.
|
|
stu said: repeating the previous comment, with the coercions this time.
(.append *buffer* ch (int start) (int length))
|
|
Converted from http://www.assembla.com/spaces/clojure/tickets/104
Attachments:
use-underscore-instead-of-dash-104.patch - https://www.assembla.com/spaces/clojure/documents/cNfYU0r9qr348xeJe5d-aX/download/cNfYU0r9qr348xeJe5d-aX
munge-genclass-field-names-104.patch - https://www.assembla.com/spaces/clojure/documents/akvHectnCr37JQeJe5dVir/download/akvHectnCr37JQeJe5dVir
|
|
richhickey said: Updating tickets (#8, #42, #113, #2, #20, #94, #96, #104, #119, #124, #127, #149, #162)
|
|
richhickey said: Updating tickets (#94, #96, #104, #119, #163)
|
|
stu said: [file:cNfYU0r9qr348xeJe5d-aX]
|
|
stu said: [file:akvHectnCr37JQeJe5dVir]
|
|
stu said: Disregard the April 14 patch, and please take a look at the April 20 patch (which includes tests).
|
|
stu said: (In [[r:084e5604710eb16df43ad529adabf18f40795bf9]]) munge genclass field names to keep Java side happy, see #104
Signed-off-by: Stuart Halloway <stu@thinkrelevance.com>
Branch: master
|