Clojure

unchecked-* functions have different behavior on primitive longs vs boxed Longs

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.7
  • Fix Version/s: Release 1.10
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Ok

Description

The behavior of unchecked-* functions such as unchecked-add, unchecked-subtract, and unchecked-multiply give different results for primitive longs (expected) and boxed longs (can get overflow exceptions). For example:

user=> *clojure-version*
{:major 1, :minor 7, :incremental 0, :qualifier nil}
user=> (doc unchecked-multiply)
-------------------------
clojure.core/unchecked-multiply
([x y])
  Returns the product of x and y, both long.
  Note - uses a primitive operator subject to overflow.
nil
user=> (unchecked-multiply 2432902008176640000 21)
-4249290049419214848
user=> (unchecked-multiply 2432902008176640000 (Long. 21))

ArithmeticException integer overflow  clojure.lang.Numbers.throwIntOverflow (Numbers.java:1501)

Normally no one would use explicit boxed Long arguments like in the example above, but these can easily occur, unintentionally, if the arguments to the unchecked functions are not explicitly type hinted as primitive long values.

Approach: clj-1832.patch

Screened: Alex Miller

Activity

Hide
Alex Miller added a comment -

I think this is a reasonable complaint. The trickiness of handling is of course doing it without affecting performance for the non-error case.

Show
Alex Miller added a comment - I think this is a reasonable complaint. The trickiness of handling is of course doing it without affecting performance for the non-error case.
Hide
Gary Fredericks added a comment - - edited

My first thought was that there shouldn't be any perf concern because it's just a matter of modifying lines such as this one, where the type dispatching has already been done. But maybe you're thinking that that line has to be more complex since the arguments could be of various different numeric types, not just Long and Long?

Show
Gary Fredericks added a comment - - edited My first thought was that there shouldn't be any perf concern because it's just a matter of modifying lines such as this one, where the type dispatching has already been done. But maybe you're thinking that that line has to be more complex since the arguments could be of various different numeric types, not just Long and Long?
Hide
Alex Miller added a comment -

That was a general comment. I haven't actually looked at the code changes necessary.

Show
Alex Miller added a comment - That was a general comment. I haven't actually looked at the code changes necessary.
Hide
Alexander Kiel added a comment -

This costs me an hour today.

I'm with Gary as I see no performance issue. But I see a code amount issue, because the whole tree of add, multiply ... methods has to be repeated.

I would opt for a doc amendment which explains that the unchecked-* functions only work with primitive types. User which see a need for using unchecked math certainly have no problem doing a cast if necessary.

Show
Alexander Kiel added a comment - This costs me an hour today. I'm with Gary as I see no performance issue. But I see a code amount issue, because the whole tree of add, multiply ... methods has to be repeated. I would opt for a doc amendment which explains that the unchecked-* functions only work with primitive types. User which see a need for using unchecked math certainly have no problem doing a cast if necessary.
Hide
Gary Fredericks added a comment -

It's probably also worth mentioning that speed is not the only use case for unchecked operations – sometimes, e.g. with crypto algorithms, you actually want the weirder kind of arithmetic, and might not want to bother with primitives at first.

Show
Gary Fredericks added a comment - It's probably also worth mentioning that speed is not the only use case for unchecked operations – sometimes, e.g. with crypto algorithms, you actually want the weirder kind of arithmetic, and might not want to bother with primitives at first.
Hide
Alexander Kiel added a comment -

After a suggestion from Alex Miller, I started with the implementation route here: https://github.com/alexanderkiel/clojure/tree/clj-1832 But it's still work in progress.

Show
Alexander Kiel added a comment - After a suggestion from Alex Miller, I started with the implementation route here: https://github.com/alexanderkiel/clojure/tree/clj-1832 But it's still work in progress.
Hide
Alexander Kiel added a comment -

This patch correctly implements all unchecked-* functions. It assumes that the issue exists only for longs because doubles are unchecked anyway and ratios have bigints.

Show
Alexander Kiel added a comment - This patch correctly implements all unchecked-* functions. It assumes that the issue exists only for longs because doubles are unchecked anyway and ratios have bigints.
Hide
Alex Miller added a comment -

This looks good to me but I do not see a Contributor Agreement on file for you Alexander. Can you sign one per here: http://clojure.org/community/contributing

Show
Alex Miller added a comment - This looks good to me but I do not see a Contributor Agreement on file for you Alexander. Can you sign one per here: http://clojure.org/community/contributing

People

Vote (1)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: