Clojure

Compiler loses 'loop's return type in some cases

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: Backlog
  • Fix Version/s: Release 1.7
  • Component/s: None
  • Labels:
    None
  • Environment:
    Clojure commit 9052ca1854b7b6202dba21fe2a45183a4534c501, version 1.3.0-master-SNAPSHOT
  • Approval:
    Vetted

Description

(set! *warn-on-reflection* true)
(fn [] (loop [b 0] (recur (loop [a 1] a))))

Generates the following warnings:

recur arg for primitive local: b is not matching primitive, had: Object, needed: long
Auto-boxing loop arg: b

This is interesting for several reasons. For one, if the arg to recur is a let form, there is no warning:

(fn [] (loop [b 0] (recur (let [a 1] a))))

Also, the compiler appears to understand the return type of loop forms just fine:

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

The problem can of course be worked around using an explicit cast on the loop form:

(fn [] (loop [b 0] (recur (long (loop [a 1] a)))))

Reported by leafw in IRC: http://clojure-log.n01se.net/date/2011-01-03.html#10:31

Activity

Hide
a_strange_guy added a comment - - edited

The problem is that a 'loop form gets converted into an anonymous fn that gets called immediately, when the loop is in a expression context (eg. its return value is needed, but not as the return value of a method/fn).

so

(fn [] (loop [b 0] (recur (loop [a 1] a))))

gets converted into

(fn [] (loop [b 0] (recur ((fn [] (loop [a 1] a))))))

see the code in the compiler:
http://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L5572

this conversion already bites you if you have mutable fields in a deftype and want to 'set! them in a loop

http://dev.clojure.org/jira/browse/CLJ-274

Show
a_strange_guy added a comment - - edited The problem is that a 'loop form gets converted into an anonymous fn that gets called immediately, when the loop is in a expression context (eg. its return value is needed, but not as the return value of a method/fn). so
(fn [] (loop [b 0] (recur (loop [a 1] a))))
gets converted into
(fn [] (loop [b 0] (recur ((fn [] (loop [a 1] a))))))
see the code in the compiler: http://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/Compiler.java#L5572 this conversion already bites you if you have mutable fields in a deftype and want to 'set! them in a loop http://dev.clojure.org/jira/browse/CLJ-274
Hide
Christophe Grand added a comment - - edited

loops in expression context are lifted into fns because else Hotspot doesn't optimize them.
This causes several problems:

  • type inference doesn't propagate outside of the loop[1]
  • the return value is never a primitive
  • mutable fields are inaccessible
  • surprise allocation of one closure objects each time the loop is entered.

Adressing all those problems isn't easy.
One can compute the type of the loop and emit a type hint but it works only with reference types. To make it works with primitive, primitie fns aren't enough since they return only long/double: you have to add explicit casts.
So solving the first two points can be done in a rather lccal way.
The two other points require more impacting changes, the goal would be to emit a method rather than a fn. So it means at the very least changing ObjExpr and adding a new subclassof ObjMethod.

[1] beware of CLJ-1111 when testing.

Show
Christophe Grand added a comment - - edited loops in expression context are lifted into fns because else Hotspot doesn't optimize them. This causes several problems:
  • type inference doesn't propagate outside of the loop[1]
  • the return value is never a primitive
  • mutable fields are inaccessible
  • surprise allocation of one closure objects each time the loop is entered.
Adressing all those problems isn't easy. One can compute the type of the loop and emit a type hint but it works only with reference types. To make it works with primitive, primitie fns aren't enough since they return only long/double: you have to add explicit casts. So solving the first two points can be done in a rather lccal way. The two other points require more impacting changes, the goal would be to emit a method rather than a fn. So it means at the very least changing ObjExpr and adding a new subclassof ObjMethod. [1] beware of CLJ-1111 when testing.
Hide
Alex Miller added a comment -

I don't think this is going to make it into 1.6, so removing the 1.6 tag.

Show
Alex Miller added a comment - I don't think this is going to make it into 1.6, so removing the 1.6 tag.

People

  • Assignee:
    Unassigned
    Reporter:
    Chouser
Vote (0)
Watch (3)

Dates

  • Created:
    Updated: