Clojure

Loops returning primtives are boxed even in return position

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: Release 1.4
  • Fix Version/s: Release 1.5
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code
  • Approval:
    Ok

Description

Reported here: https://groups.google.com/d/topic/clojure/atoFzbyuyos/discussion

(defn first-bit?
  {:inline (fn [n] `(== 1 (clojure.lang.Numbers/and ~n, 1)) )}
  [^long n]
  (== 1 (clojure.lang.Numbers/and n, 1)))

(defn exp-int
  ^double [^double x, ^long c]
  (loop [result 1.0, factor x, c c]
    (if (> c 0)
        (recur
         (if (first-bit? c)
           (* result factor)
           result),
         (* factor factor),
         (bit-shift-right c 1))
      result)))

Last lines of the Java bytecode of `exp-int`:

59 dload 5;               /* result */
61 invokestatic 40;       /* java.lang.Double java.lang.Double.valueOf(double c) */
64 checkcast 85;          /* java.lang.Number */
67 invokevirtual 89;      /* double doubleValue() */
70 dreturn;

The compiler doesn't currently infer the primitive type as soon as there is a recur:

(use '[clojure.contrib.repl-utils :only [expression-info]])
(expression-info '(loop [a 1] a))
;=> {:class long, :primitive? true}
(expression-info '(loop [a 1] (if true a (recur a)))
;=> nil

Patch attached.

Activity

Christophe Grand made changes -
Field Original Value New Value
Description Reported here: https://groups.google.com/d/topic/clojure/atoFzbyuyos/discussion

(defn first-bit?
  {:inline (fn [n] `(== 1 (clojure.lang.Numbers/and ~n, 1)) )}
  [^long n]
  (== 1 (clojure.lang.Numbers/and n, 1)))

(defn exp-int
  ^double [^double x, ^long c]
  (loop [result 1.0, factor x, c c]
    (if (> c 0)
        (recur
         (if (first-bit? c)
           (* result factor)
           result),
         (* factor factor),
         (bit-shift-right c 1))
      result)))

Last lines of the Java bytecode of `exp-int`:
59 dload 5; /* result */
61 invokestatic 40; /* java.lang.Double java.lang.Double.valueOf(double c) */
64 checkcast 85; /* java.lang.Number */
67 invokevirtual 89; /* double doubleValue() */
70 dreturn;

The compiler doesn't currently infer the primitive type as soon as there is a recur:

(use '[clojure.contrib.repl-utils :only [expression-info]])
(expression-info '(loop [a 1] a))
;=> {:class long, :primitive? true}
(expression-info '(loop [a 1] (if true a (recur a)))
;=> nil

Patch attached.
Reported here: https://groups.google.com/d/topic/clojure/atoFzbyuyos/discussion

{noformat}
(defn first-bit?
  {:inline (fn [n] `(== 1 (clojure.lang.Numbers/and ~n, 1)) )}
  [^long n]
  (== 1 (clojure.lang.Numbers/and n, 1)))

(defn exp-int
  ^double [^double x, ^long c]
  (loop [result 1.0, factor x, c c]
    (if (> c 0)
        (recur
         (if (first-bit? c)
           (* result factor)
           result),
         (* factor factor),
         (bit-shift-right c 1))
      result)))
{noformat}

Last lines of the Java bytecode of `exp-int`:
{noformat}
59 dload 5; /* result */
61 invokestatic 40; /* java.lang.Double java.lang.Double.valueOf(double c) */
64 checkcast 85; /* java.lang.Number */
67 invokevirtual 89; /* double doubleValue() */
70 dreturn;
{noformat}

The compiler doesn't currently infer the primitive type as soon as there is a recur:

{noformat}
(use '[clojure.contrib.repl-utils :only [expression-info]])
(expression-info '(loop [a 1] a))
;=> {:class long, :primitive? true}
(expression-info '(loop [a 1] (if true a (recur a)))
;=> nil
{noformat}

Patch attached.
Hide
Stuart Halloway added a comment -

Tests pass, code looks reasonable. Would appreciate additional review.

Show
Stuart Halloway added a comment - Tests pass, code looks reasonable. Would appreciate additional review.
Stuart Halloway made changes -
Approval Screened [ 10004 ]
Hide
Timothy Baldridge added a comment -

Tests also pass here. Looked through the code and played with a patched version of Clojure. I can't see a problem with it.

Show
Timothy Baldridge added a comment - Tests also pass here. Looked through the code and played with a patched version of Clojure. I can't see a problem with it.
Hide
Christophe Grand added a comment -

FYI, gvec.clj has two loops which return primitives and thus was formerly boxed.

Show
Christophe Grand added a comment - FYI, gvec.clj has two loops which return primitives and thus was formerly boxed.
Christophe Grand made changes -
Assignee Christophe Grand [ cgrand ]
Rich Hickey made changes -
Approval Screened [ 10004 ] Ok [ 10007 ]
Fix Version/s Release 1.5 [ 10150 ]
Stuart Halloway made changes -
Resolution Completed [ 1 ]
Status Open [ 1 ] Closed [ 6 ]

People

Vote (3)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: