<< Back to previous view

[CLJ-976] Implement reader literal and print support for PersistentQueue data structure Created: 27/Apr/12  Updated: 22/Aug/13

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

Type: Enhancement Priority: Minor
Reporter: Fogus Assignee: Fogus
Resolution: Unresolved Votes: 4
Labels: data-structures, queue, reader, tagged-literals

Attachments: File CLJ-976-queue-literal-eval-and-synquote.diff     Text File clj-976-queue-literal-eval-and-synquote-patch-v3.txt     File CLJ-976-queue-literal-eval.diff    
Patch: Code and Test
Approval: Triaged

 Description   

Clojure's PersistentQueue structure has been in the language for quite some time now and has found its way into a fair share of codebases. However, the creation of queues is a two step operation often of the form:

(conj clojure.lang.PersistentQueue/EMPTY :a :b :c)

;=> #<PersistentQueue clojure.lang.PersistentQueue@78d5f6bc>

A better experience might be the following:

#queue [:a :b :c]

;=> #queue [:a :b :c]

(pop #queue [:a :b :c])

;=> #queue [:b :c]

This syntax is proposed and discussed in the Clojure-dev group at https://groups.google.com/forum/?fromgroups#!topic/clojure-dev/GQqus5Wycno

Open question: Should the queue literal's arguments eval? The implications of this are illustrated below:

;; non-eval case
#queue [1 2 (+ 1 2)]

;=> #queue [1 2 (+ 1 2)]


;; eval case
#queue [1 2 (+ 1 2)]

;=> #queue [1 2 3]

The answer to this open question will determine the implementation.



 Comments   
Comment by Steve Miner [ 27/Apr/12 10:18 AM ]

I think the non-eval behavior would be consistent with the other reader literals in Clojure 1.4. It's definitely better for interop where some other language implementation could be expected to handle a few literal representations, but not the evaluation of Clojure expressions. Use a regular function if the args need evaluation.

Comment by Chas Emerick [ 27/Apr/12 10:19 AM ]

The precedent of records seems relevant:

=> (defrecord A [b])
user.A
=> #user.A[(+ 4 5)]
#user.A{:b (+ 4 5)}
=> #user.A{:b (+ 4 5)}
#user.A{:b (+ 4 5)}

This continues to make sense, as otherwise queues would need to print with an extra (quote …) form around lists — which records neatly avoid:

=> (A. '(+ 4 5))
#user.A{:b (+ 4 5)}

Does this mean that a queue fn (analogous to vector, maybe) will also make an appearance? It'd be handy for HOF usage.

Comment by Fogus [ 27/Apr/12 11:00 AM ]

Added a patch for the tagged literal support ONLY. This is only one part of the total solution. This provides the read-string and printing capability. I'd like more discussion around the eval side before I get dive into the compiler.

Comment by Paul Michael Bauer [ 27/Apr/12 6:45 PM ]

In addition to Chas' observations on consistency with record literals, would eval in queue literals open up the same security hole as #=, needing to respect *read-eval*?
When needing eval inside a queue literal, embedding a #= seems more apropos.

Comment by Fogus [ 04/May/12 1:14 PM ]

Evalable queue literal support.

Comment by Andy Fingerhut [ 10/May/12 5:54 PM ]

Neither of the patches CLJ-976-queue-literal-tagged-parse-support-only.diff dated Apr 27, 2012 nor CLJ-976-queue-literal-eval.diff dated May 4, 2012 apply cleanly to latest master as of May 10, 2012.

Comment by Fogus [ 11/May/12 10:15 AM ]

Updated patch file to merge with latest master.

Comment by Fogus [ 20/Jul/12 1:14 PM ]

New patch with support fixed for syntax-quote.

Comment by Stuart Sierra [ 17/Aug/12 12:41 PM ]

Patch does not apply as of commit f5f4faf95051f794c9bfa0315e4457b600c84cef

Comment by Fogus [ 17/Aug/12 3:06 PM ]

Weird. I was able to download the CLJ-976-queue-literal-eval-and-synquote.diff patch and apply it to HEAD as of just now (f5f4faf95051f794c9bfa0315e4457b600c84cef). There were whitespace warnings, but the patch applied, compiles and passes all tests.

Comment by Andy Fingerhut [ 17/Aug/12 7:29 PM ]

With latest head I was able to successfully apply patch CLJ-976-queue-literal-eval-and-synquote.diff with this command:

git am --keep-cr -s < CLJ-976-queue-literal-eval-and-synquote.diff

with some warnings, but successfully applied. If I try it without the --keep-cr option, the patch fails to apply. I believe this is often a sign that either one of the files being patched, or the patch itself, contains CR/LF line endings, which some of the Clojure source files definitely do.

The command above (with --keep-cr) is currently the one recommended for applying patches on page http://dev.clojure.org/display/design/JIRA+workflow in section "Screening Tickets". I added the suggested --keep-cr option after running across another patch that applied with the option, but not without it.

Comment by Andy Fingerhut [ 28/Aug/12 5:45 PM ]

Presumptuously changing Approval from Incomplete back to Test, since the latest patch does apply cleanly if --keep-cr option is used.

Comment by Rich Hickey [ 08/Sep/12 6:48 AM ]

this needs more time

Comment by Fogus [ 18/Sep/12 8:15 AM ]

Rich,

Do you mind providing a little more detail? I would be happy to make any changes if needed. However, if it's just a matter of its relationship to EDN and/or waiting until the next release then I am happy to wait. In either case, I'd like to complete this or push it to the back of my mind. Thanks.

Comment by Andy Fingerhut [ 05/Oct/12 7:49 AM ]

clj-976-queue-literal-eval-and-synquote-patch-v2.txt dated Oct 5 2012 is identical to Fogus's patch CLJ-976-queue-literal-eval-and-synquote.diff dated Jul 20 2012. It simply removes one line addition to clojure.iml that Rich has since added in a different commit, so that this patch now applies cleanly to latest master.

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

clj-976-queue-literal-eval-and-synquote-patch-v3.txt dated oct 16 2012 is identical to Fogus's patch CLJ-976-queue-literal-eval-and-synquote.diff dated Jul 20 2012. It simply removes one line addition to clojure.iml that Rich has since added in a different commit, so that this patch now applies cleanly to latest master.

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

Fogus, with the recent commit of a patch for CLJ-1070, my touched-up patch clj-976-queue-literal-eval-and-synquote-patch-v3.txt dated Oct 16 2012 doesn't apply cleanly. In this case it isn't simply a few lines of context that have changed, it is the interfaces that PersistentQueue implements have been changed. It might be best if you take a look at the latest code and the patch and consider how it should be updated.

Comment by Steve Miner [ 06/Apr/13 8:07 AM ]

Related to CLJ-1078.

Comment by Alex Miller [ 22/Aug/13 10:38 PM ]

Moving back to Triaged as Rich has not vetted.





Generated at Wed Nov 26 12:55:28 CST 2014 using JIRA 4.4#649-r158309.