<< Back to previous view

[CLJ-827] unsigned-bit-shift-right Created: 09/Aug/11  Updated: 22/Nov/13  Resolved: 22/Nov/13

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

Type: Enhancement Priority: Major
Reporter: Joe Gallo Assignee: Unassigned
Resolution: Completed Votes: 11
Labels: math

Attachments: Text File 0001-add-unsigned-bit-shift-right.patch     Text File 0001-CLJ-827-Add-bit-shift-right-logical.patch     Text File clj-827-unsigned-bit-shift-right-with-tests.patch    
Patch: Code and Test
Approval: Ok


Add a clojure equivalent of >>>.

The patch implements a new unsigned-bit-shift-right function that has the semantics of Java long >>>. This differs in particular in the handling of the shift distance which uses the least significant 6 bits of the shift distance (thus always in the range 0-63 inclusive). A Java method Numbers.unsignedShiftRightInt is included but not used by the surfaced function.

Patch: clj-827-unsigned-bit-shift-right-with-tests.patch

Screened by: Alex Miller

Related ticket: Ticket CLJS-514 has been created to track the future work of making ClojureScript's name for this operation the same as the one used in Clojure, after this ticket is completed. David Nolen is agreeable to making this change, even though it would rename an already-defined public function.

Comment by Joe Gallo [ 11/Nov/11 12:58 PM ]

I just realized (with the asssistance of Paul Stadig) that just doing only longs is probably sufficient, as you can get the integer version if you really want it:

> (int (bit-and Integer/MAX_VALUE (unsigned-bit-shift-right -5 1)))

Of course, that's less efficient than just doing it directly with java, but it's enough that I think my concern from the previous comment is addressed.

Comment by Tim McCormack [ 16/Jan/12 6:01 PM ]

I have attached "0001-CLJ-827-Add-bit-shift-right-logical.patch", which implements a logical bit-shift-right using the same JVM bytecode as >>>.

The patch mimics the implementations of << and >>.

Comment by Stuart Halloway [ 02/Feb/13 5:09 PM ]

For context, this feature appears to be needed for Clojure-in-Clojure data structures: https://groups.google.com/d/msg/clojure-dev/iAwH7CLSFzE/6wzDH4RS1YQJ

Comment by Michał Marczyk [ 08/Feb/13 5:31 AM ]

Just wanted to note that I've introduced this operation to ClojureScript when implementing PersistentHashMap. The name over there is bit-shift-right-zero-fill. Would it be alright for Clojure to use that name? Failing that, ClojureScript would probably have to change to match.

Comment by Gabriel Horner [ 24/May/13 2:23 PM ]

I've added clj-827-unsigned-bit-shift-right-with-tests.patch which builds off of Tim's patch and adds tests. I also renamed the new fn to unsigned-bit-shift-right after chatting with Rich.

@Michal - This may mean you need to rename the cljs fn. After this lands on master, I can open a ticket with a patch if you'd like.

Comment by Michał Marczyk [ 31/May/13 3:43 PM ]

@Gabriel: Thanks! bit-shift-right-zero-fill is not marked private in ClojureScript, so it'll be a breaking change; if it's going to happen eventually (that is, if the name is settled), I suppose it's best to make it without waiting for Clojure.

At any rate, I went ahead and created CLJS-514 to track the coordination effort. No patch attached; if you'd like to do the patching on both sides, that would be cool, otherwise I'll be happy to provide the CLJS patch.

Comment by Gabriel Horner [ 03/Jun/13 10:10 AM ]

@Michal - I didn't mark it private. Are we taking about the same with-tests.patch? I'll try to get around to the cljs patch but feel free to beat me to it if I'm slow.

Comment by Michał Marczyk [ 10/Jun/13 1:57 AM ]

@Gabriel: I mean it's not marked private in ClojureScript, so ClojureScript already exports the ...-zero-fill name, technically speaking. As per David Nolen's comment on CLJS-514, this is probably not a big deal.

Comment by Alex Miller [ 23/Aug/13 8:24 AM ]

It's unclear to me what patches are in play and whether the Clojurescript question is resolved in this ticket. Can someone clean it up? Moving to Incomplete.

Comment by Michał Marczyk [ 23/Aug/13 9:02 AM ]

AFAICT the newest two patches are relevant (Gabriel's patch – the latest one – applies on top of Tim's patch – second latest); the first one must have been left around as per the usual Clojure JIRA custom to leave historical patches in place.

As for ClojureScript, Gabriel's comment indicates that Rich expressed a preference for the name unsigned-bit-shift-right. ClojureScript can adjust to that no problem, in fact I think we should just switch the name ahead of time. There is no open question on the ClojureScript side except if Clojure actually merged the patches we'd be able to make any final adjustments and close the CLJS ticket.

NB. the adjustments to make there are only about the name of the function anyway, as explained in the comments above.

I'll also note that the present couple of patches (1) touch no existing behaviour, (2) add in a missing basic and useful operation which we cannot just simulate in Clojure if performance is a concern at all. Hence my long-standing +1 on this issue.

Comment by Andy Fingerhut [ 23/Aug/13 11:52 AM ]

I made some additions to the description on the one patch that I think should be considered, and why, and comments on what appears to be the current state of the ClojureScript question. Feel free to edit further if it needs it.

Comment by Alex Miller [ 11/Oct/13 12:54 PM ]

For reference, notes on how Java does >>> for ints and longs is here:

I found that section particularly helpful wrt understanding the behavior of the shift distance parameter. Namely, that for longs the shift distance uses only the least significant 6 bits (bit-and with 0x3f) which explains some of the added tests using large or negative shift distances.

Generated at Fri Apr 18 13:13:59 CDT 2014 using JIRA 4.4#649-r158309.