### [CLJ-457] lazy recursive definition giving incorrect results Created: 13/Oct/10  Updated: 03/Aug/15

Status: Reopened
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: None

 Type: Defect Priority: Major Reporter: Assembla Importer Assignee: Christophe Grand Resolution: Unresolved Votes: 0 Labels: None

 Attachments: CLJ-457-2.diff     clj-457-3.diff Patch: Code and Test

 Description
 If you define a global data var in terms of a lazy sequence referring to that same var, you can get different results depending on the chunkiness of laziness of the functions being used to build the collection. Clojure's lazy sequences don't promise to support this, but they shouldn't return wrong answers. In the example given in http://groups.google.com/group/clojure/browse_thread/thread/1c342fad8461602d (and repeated below), Clojure should not return bad data. An error message would be good, and even an infinite loop would be more reasonable than the current behavior. (Similar issue reported here: https://groups.google.com/d/topic/clojure/yD941fIxhyE/discussion) ```(def nums (drop 2 (range Long/MAX_VALUE))) (def primes (cons (first nums) (lazy-seq (->> (rest nums) (remove (fn [x] (let [dividors (take-while #(<= (* % %) x) primes)] (println (str "primes = " primes)) (some #(= 0 (rem x %)) dividors)))))))) (take 5 primes) It prints out: (primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) primes = (2) 2 3 5 7 9)```

Comment by Assembla Importer [ 13/Oct/10 3:00 PM ]

Converted from http://www.assembla.com/spaces/clojure/tickets/457

Comment by Aaron Bedra [ 10/Dec/10 9:08 AM ]

Stu and Rich talked about making this an error, but it would break some existing code to do so.

Comment by Rich Hickey [ 17/Dec/10 8:03 AM ]

Is there a specific question on this?

Comment by Aaron Bedra [ 05/Jan/11 9:05 PM ]

Stu, you and I went over this but I can't remember exactly what the question was here.

Comment by Christophe Grand [ 28/Nov/12 12:24 PM ]

Tentative patch attached.
Have you an example of existing code which is broken by such a patch (as mention by Aaron Bedra)?

Comment by Rich Hickey [ 30/Nov/12 9:43 AM ]

The patch intends to do what? We have only a problem description and code. Please enumerate the plan rather than make us decipher the patch.

As a first principle, I don't want Clojure to promise that such recursively defined values are possible.

Comment by Christophe Grand [ 30/Nov/12 10:23 AM ]

The proposal here is to catch recursive seq realization (ie when computing the body of a lazy-seq attempts to access the same seq) and throw an exception.

Currently when such a case happens, the recursive access to the seq returns nil. This results in incorrect code seemingly working but producing incorrect results or even incorrect code producing correct results out of luck (see https://groups.google.com/d/topic/clojure/yD941fIxhyE/discussion for such an example).

So this patch moves around the modification to the LazySeq state (f, sv and s fields) before all potentially recursive method call (.sval in the while of .seq and .invoke in .sval) so that, upon reentrance, the state of the LazySeq is coherent and able to convey the fact the seq is already being computed.

Currently a recursive call may find f and sv cleared and concludes the computation is done and the result is in s despite s being unaffected yet.

Currently:

State f sv s
Unrealized not null null null
Realized null null anything
Being realized/recursive call from fn.invoke not null null null
Being realized/recursive call from ls.sval null null null

Note that "Being realized" states overlap with Unrealized or Realized.
(NB: "anything" includes null)

With the patch:

State f sv s
Unrealized not null null null
Realized null null anything but this
Being realized null null this
Comment by Andy Fingerhut [ 30/Nov/12 2:06 PM ]

That last comment, Christophe, goes a long way to explaining the idea to me, at least. Any chance comments with similar content could be added as part of the patch?

Comment by Christophe Grand [ 03/Dec/12 11:18 AM ]

New patch with a comment explaining the expected states.
Note: I tidied the states table up.

```// Before calling user code (f.invoke() in sval and, indirectly,
// ((LazySeq)ls).sval() in seq -- and even RT.seq() in seq), ensure that
// the LazySeq state is in one of these states:
//
// State            f          sv
// ================================
// Unrealized       not null   null
// Realized         null       null
// Being realized   null       this```

CLJ-1119 is also fixed by this patch.

Comment by Andy Fingerhut [ 23/Nov/13 12:35 AM ]

Patch clj-457-3.diff is identical to Christophe's CLJ-457-2.diff, except it has been updated to no longer conflict with the commit made for CLJ-949 on Nov 22, 2013, which basically removed the try/catch in method sval(). It passes tests, and I don't see anything wrong with it, but it would be good if Christophe could give it a look, too.

Comment by Alex Miller [ 02/Aug/15 7:46 PM ]

No longer reproducible

Comment by Nicola Mometto [ 03/Aug/15 10:01 AM ]

Alex, the posted example is no longer reproducible because `(range)` does not produce a chunked-seq anymore.
OTOH `(range n)` does so replacing `(range)` with something like `(range Long/MAX_VALUE)` in the code example in the ticket description is sufficient to resurface the bug:

```Clojure 1.8.0-master-SNAPSHOT
user=> (def nums (drop 2 (range Long/MAX_VALUE)))
#'user/nums
(def primes (cons (first nums)
(lazy-seq (->>
(rest nums)
(remove
(fn [x]
(let [dividors (take-while #(<= (* % %) x)
primes)]
(println (str "primes = " primes))
(some #(= 0 (rem x %)) dividors))))))))
#'user/primes
user=> (take 5 primes)
(primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
primes = (2)
2 3 5 7 9)```

### [CLJ-1096] Make destructuring emit direct keyword lookups Created: 29/Oct/12  Updated: 11/Jan/16

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

 Type: Enhancement Priority: Major Reporter: Christophe Grand Assignee: Christophe Grand Resolution: Unresolved Votes: 4 Labels: destructuring, performance

 Attachments: desctructure-keyword-lookup.diff     inline-get-keyword.diff Patch: Code Approval: Triaged

 Description
 Currently associative destructuring emits calls to get. The attached patch modify desctruture to emit direct keyword lookups when possible.

 Comment by Christophe Grand [ 04/Sep/13 3:40 AM ] Rethinking about this patch now, it may be too specific: get's inline expansion should be modified when the key is a literal keyword. Comment by Christophe Grand [ 04/Sep/13 3:41 AM ] More generic patch (inline-get-keyword.diff): all get calls with literal keywords as keys are inlined to direct keyword lookup. Comment by John Hume [ 19/May/14 1:14 PM ] Is this only stalled out of lack of interest? Comment by Andy Fingerhut [ 19/May/14 6:13 PM ] There are currently about 50 tickets "triaged", i.e. marked for Rich to look at and decide whether they are things he is interested in seeing a patch for, and another 25 or so that were triaged and he has "vetted" them, and they are in various stages of having patches written for them, screened, etc. That doesn't mean anything for this ticket in particular – just wanted to make it clear that there are a bunch of other tickets that are getting some attention, and a bunch of others that are not. What gets triaged depends somewhat upon how severe the issue appears. You can vote on the ticket, and try to persuade others to do so as well, if they think this would enhance the performance of some commonly-written types of Clojure code. You could also consider doing some benchmarking with & without these patches to see how much performance they can gain.

### [CLJ-326] add :as-of option to refer Created: 30/Apr/10  Updated: 26/Jul/13

Status: In Progress
Project: Clojure
Component/s: None
Affects Version/s: None
Fix Version/s: Backlog

 Type: Enhancement Priority: Minor Reporter: Christophe Grand Assignee: Christophe Grand Resolution: Unresolved Votes: 0 Labels: None

 Approval: Vetted

 Description
 :as-of allows library authors to specify a known subset of vars to refer from clojure (or any other library which would use :added metadata). (ns foo (:refer-clojure :as-of "1.1")) is equivalent to (ns foo (:refer-clojure :only [public-documented-vars-which-already-existed-in-1.1]))

 Comment by Assembla Importer [ 24/Aug/10 10:19 AM ] Converted from http://www.assembla.com/spaces/clojure/tickets/326 Attachments: add-as-of-to-refer.patch - https://www.assembla.com/spaces/clojure/documents/a8SumUvcOr37SmeJe5cbLA/download/a8SumUvcOr37SmeJe5cbLA Comment by Assembla Importer [ 24/Aug/10 10:19 AM ] cgrand said: [file:a8SumUvcOr37SmeJe5cbLA]: requires application of #325 Comment by Assembla Importer [ 24/Aug/10 10:19 AM ] richhickey said: Do we still need this?

Generated at Thu Mar 23 21:25:21 CDT 2017 using JIRA 4.4#649-r158309.