Fix stack overflow resulting from co-recursive lazy seqs (#632)

* Fix stack overflow resulting from co-recursive lazy seqs

* Writing tests is good

* Queues need tests too

* Documentation

* Re-documentation

* Slight updates to severals of things

* Co-recursion test

Author: chrisrink10

src/basilisp/core.lpy

@@ -2302,18 +2302,17 @@
    coll))
 
 (defn iterate
-  "Returns a lazy sequence of (f x), (f (f x)) and so on."
+  "Returns a lazy sequence of x, (f x), (f (f x)) and so on."
   [f x]
   (lazy-seq
-   (let [v (f x)]
-     (when v
-       (cons v (iterate f v))))))
+   (when x
+     (cons x (iterate f (f x))))))
 
 (defn range
   "Return a range of integers from start. If end is specified, the
   sequence will terminate at end."
   ([]
-   (iterate inc -1))
+   (iterate inc 0))
   ([end]
    (lazy-seq (cons 0 (range 1 end))))
   ([start end]
@@ -2328,8 +2327,12 @@
   "Return a function which returns the logical complement of the return
   value of (apply f args)."
   [f]
-  (fn [& args]
-    (not (apply f args))))
+  (fn
+    ([x] (not (f x)))
+    ([x y] (not (f x y)))
+    ([x y z] (not (f x y z)))
+    ([x y z & args]
+     (not (apply f x y z args)))))
 
 (defn constantly
   "Returns a function that accepts any number of arguments and returns x."
@@ -2674,8 +2677,8 @@
             (ensure-reduced result)))))))
   ([n coll]
    (lazy-seq
-    (when (seq coll)
-      (when (> n 0)
+    (when (> n 0)
+      (when (seq coll)
         (cons (first coll) (take (dec n) (rest coll))))))))
 
 (defn take-while

src/basilisp/lang/compiler/generator.py

@@ -169,6 +169,12 @@ def new_frame(self, name: str, is_context_boundary: bool):
         """Context manager for creating a new stack frame."""
         yield SymbolTable(name, is_context_boundary=is_context_boundary, parent=self)
 
+    @property
+    def is_top(self) -> bool:
+        """Return true if this is the top-level symbol table (e.g. there is no
+        parent)."""
+        return self._parent is None
+
     @property
     def context_boundary(self) -> "SymbolTable":
         """Return the nearest context boundary parent symbol table to this one. If the
@@ -1494,10 +1500,18 @@ def _synthetic_do_to_py_ast(ctx: GeneratorContext, node: Do) -> GeneratedPyAST:
 MetaNode = Union[Const, MapNode]
 
 
-def __fn_name(s: Optional[str]) -> str:
+def __fn_name(ctx: GeneratorContext, s: Optional[str]) -> str:
     """Generate a safe Python function name from a function name symbol.
-    If no symbol is provided, generate a name with a default prefix."""
-    return genname("__" + munge(Maybe(s).or_else_get(_FN_PREFIX)))
+
+    If no symbol is provided, generate a name with a default prefix.
+
+    Prepends the name of the parent symbol table (if one exists) to help with debugging
+    and readability of stack traces."""
+    ctx_boundary = ctx.symbol_table.context_boundary
+    ctx_boundary_prefix = "" if ctx_boundary.is_top else f"{ctx_boundary.name}__"
+    return genname(
+        munge("".join(("__", ctx_boundary_prefix, Maybe(s).or_else_get(_FN_PREFIX))))
+    )
 
 
 def __fn_args_to_py_ast(
@@ -1619,7 +1633,7 @@ def __single_arity_fn_to_py_ast(
     assert method.op == NodeOp.FN_ARITY
 
     lisp_fn_name = node.local.name if node.local is not None else None
-    py_fn_name = __fn_name(lisp_fn_name) if def_name is None else munge(def_name)
+    py_fn_name = __fn_name(ctx, lisp_fn_name) if def_name is None else munge(def_name)
     py_fn_node = ast.AsyncFunctionDef if node.is_async else ast.FunctionDef
     with ctx.new_symbol_table(
         py_fn_name, is_context_boundary=True
@@ -1859,7 +1873,7 @@ def __multi_arity_fn_to_py_ast(  # pylint: disable=too-many-locals
     assert node.kwarg_support is None, "multi-arity functions do not support kwargs"
 
     lisp_fn_name = node.local.name if node.local is not None else None
-    py_fn_name = __fn_name(lisp_fn_name) if def_name is None else munge(def_name)
+    py_fn_name = __fn_name(ctx, lisp_fn_name) if def_name is None else munge(def_name)
 
     py_fn_node = ast.AsyncFunctionDef if node.is_async else ast.FunctionDef
 

src/basilisp/lang/interfaces.py

@@ -172,7 +172,7 @@ class ISeqable(Iterable[T]):
     __slots__ = ()
 
     @abstractmethod
-    def seq(self) -> "ISeq[T]":
+    def seq(self) -> "Optional[ISeq[T]]":
         raise NotImplementedError()
 
 
@@ -297,7 +297,7 @@ def cons(self: T_vec, *elems: T) -> T_vec:  # type: ignore[override]
         raise NotImplementedError()
 
     @abstractmethod
-    def seq(self) -> "ISeq[T]":  # type: ignore[override]
+    def seq(self) -> "Optional[ISeq[T]]":  # type: ignore[override]
         raise NotImplementedError()
 
 
@@ -481,7 +481,7 @@ def rest(self) -> "ISeq[T]":
     def cons(self, elem: T) -> "ISeq[T]":
         raise NotImplementedError()
 
-    def seq(self) -> "ISeq[T]":
+    def seq(self) -> "Optional[ISeq[T]]":
         return self
 
     def _lrepr(self, **kwargs):

src/basilisp/lang/list.py

@@ -73,6 +73,11 @@ def cons(self, *elems: T) -> "PersistentList[T]":
     def empty() -> "PersistentList":
         return EMPTY
 
+    def seq(self) -> Optional[ISeq[T]]:
+        if len(self._inner) == 0:
+            return None
+        return super().seq()
+
     def peek(self):
         return self.first
 

src/basilisp/lang/map.py

@@ -256,7 +256,9 @@ def cons(  # type: ignore[override]
     def empty() -> "PersistentMap":
         return EMPTY
 
-    def seq(self) -> ISeq[IMapEntry[K, V]]:
+    def seq(self) -> Optional[ISeq[IMapEntry[K, V]]]:
+        if len(self._inner) == 0:
+            return None
         return sequence(MapEntry.of(k, v) for k, v in self._inner.items())
 
     def to_transient(self) -> TransientMap[K, V]:

src/basilisp/lang/queue.py

@@ -75,7 +75,9 @@ def pop(self) -> "PersistentQueue[T]":
             raise IndexError("Cannot pop an empty queue")
         return PersistentQueue(self._inner.popleft())
 
-    def seq(self) -> ISeq[T]:
+    def seq(self) -> Optional[ISeq[T]]:
+        if len(self._inner) == 0:
+            return None
         return sequence(self)
 
 

src/basilisp/lang/reader.py

@@ -965,7 +965,9 @@ def _walk(inner_f, outer_f, form):
     elif isinstance(form, IPersistentVector):
         return outer_f(vec.vector(map(inner_f, form)))
     elif isinstance(form, IPersistentMap):
-        return outer_f(lmap.hash_map(*chain.from_iterable(map(inner_f, form.seq()))))
+        return outer_f(
+            lmap.hash_map(*chain.from_iterable(map(inner_f, form.seq() or ())))
+        )
     elif isinstance(form, IPersistentSet):
         return outer_f(lset.set(map(inner_f, form)))
     else:

src/basilisp/lang/seq.py

@@ -14,18 +14,26 @@
 
 
 class _EmptySequence(IWithMeta, ISequential, ISeq[T]):
+    __slots__ = ("_meta",)
+
+    def __init__(self, meta: Optional[IPersistentMap] = None):
+        self._meta = meta
+
     def __repr__(self):
         return "()"
 
     def __bool__(self):
         return True
 
+    def seq(self) -> Optional[ISeq[T]]:
+        return None
+
     @property
     def meta(self) -> Optional[IPersistentMap]:
-        return None
+        return self._meta
 
-    def with_meta(self, meta: Optional[IPersistentMap]) -> "Cons[T]":  # type: ignore
-        return Cons(meta=meta)
+    def with_meta(self, meta: Optional[IPersistentMap]) -> "_EmptySequence[T]":
+        return _EmptySequence(meta=meta)
 
     @property
     def is_empty(self) -> bool:
@@ -51,7 +59,7 @@ class Cons(ISeq[T], ISequential, IWithMeta):
 
     def __init__(
         self,
-        first=None,
+        first,
         seq: Optional[ISeq[T]] = None,
         meta: Optional[IPersistentMap] = None,
     ) -> None:
@@ -79,7 +87,7 @@ def meta(self) -> Optional[IPersistentMap]:
         return self._meta
 
     def with_meta(self, meta: Optional[IPersistentMap]) -> "Cons[T]":
-        return Cons(first=self._first, seq=self._rest, meta=meta)
+        return Cons(self._first, seq=self._rest, meta=meta)
 
 
 class _Sequence(IWithMeta, ISequential, ISeq[T]):
@@ -141,19 +149,23 @@ def cons(self, elem):
 class LazySeq(IWithMeta, ISequential, ISeq[T]):
     """LazySeqs are wrappers for delaying sequence computation. Create a LazySeq
     with a function that can either return None or a Seq. If a Seq is returned,
-    the LazySeq is a proxy to that Seq."""
+    the LazySeq is a proxy to that Seq.
 
-    __slots__ = ("_gen", "_seq", "_meta")
+    Callers should never provide the `obj` or `seq` arguments -- these are provided
+    only to support `with_meta` returning a new LazySeq instance."""
+
+    __slots__ = ("_gen", "_obj", "_seq", "_meta")
 
-    # pylint:disable=assigning-non-slot
     def __init__(
         self,
         gen: Optional[LazySeqGenerator],
+        obj: Optional[ISeq[T]] = None,
         seq: Optional[ISeq[T]] = None,
         *,
         meta: Optional[IPersistentMap] = None,
     ) -> None:
         self._gen: Optional[LazySeqGenerator] = gen
+        self._obj: Optional[ISeq[T]] = obj
         self._seq: Optional[ISeq[T]] = seq
         self._meta = meta
 
@@ -162,32 +174,72 @@ def meta(self) -> Optional[IPersistentMap]:
         return self._meta
 
     def with_meta(self, meta: Optional[IPersistentMap]) -> "LazySeq[T]":
-        return LazySeq(self._gen, seq=self._seq, meta=meta)
-
-    # pylint:disable=assigning-non-slot
-    def _realize(self):
+        return LazySeq(self._gen, obj=self._obj, seq=self._seq, meta=meta)
+
+    # LazySeqs have a fairly complex inner state, in spite of the simple interface.
+    # Calls from Basilisp code should be providing the only generator seed function.
+    # Calls to `(seq ...)` cause the LazySeq to cache the generator function locally
+    # (as explained in _compute_seq), clear the _gen attribute, and cache the results
+    # of that generator function call as _obj. _obj may be None, some other ISeq, or
+    # perhaps another LazySeq. Finally, the LazySeq attempts to consume all returned
+    # LazySeq objects before calling `(seq ...)` on the result, which is cached in the
+    # _seq attribute.
+
+    def _compute_seq(self) -> Optional[ISeq[T]]:
         if self._gen is not None:
-            self._seq = to_seq(self._gen())
+            # This local caching of the generator function and clearing of self._gen
+            # is absolutely critical for supporting co-recursive lazy sequences.
+            #
+            # The original example that prompted this change is below:
+            #
+            #   (def primes (remove
+            #                (fn [x] (some #(zero? (mod x %)) primes))
+            #                (iterate inc 2)))
+            #
+            #   (take 5 primes)  ;; => stack overflow
+            #
+            # If we don't clear self._gen, each successive call to (some ... primes)
+            # will end up forcing the primes LazySeq object to call self._gen, rather
+            # than caching the results, allowing examination of the partial seq
+            # computed up to that point.
+            gen = self._gen
             self._gen = None
+            self._obj = gen()
+        return self._obj if self._obj is not None else self._seq
+
+    def seq(self) -> Optional[ISeq[T]]:
+        self._compute_seq()
+        if self._obj is not None:
+            o = self._obj
+            self._obj = None
+            # Consume any additional lazy sequences returned immediately so we have a
+            # "real" concrete sequence to proxy to.
+            #
+            # The common idiom with LazySeqs is to return (cons value (lazy-seq ...))
+            # from the generator function, so this will only result in evaluating away
+            # instances where _another_ LazySeq is returned rather than a cons cell
+            # with a concrete first value. This loop will not consume the LazySeq in
+            # the rest position of the cons.
+            while isinstance(o, LazySeq):
+                o = o._compute_seq()  # type: ignore
+            self._seq = to_seq(o)
+        return self._seq
 
     @property
     def is_empty(self) -> bool:
-        self._realize()
-        return self._seq is None
+        return self.seq() is None
 
     @property
     def first(self) -> Optional[T]:
-        self._realize()
         try:
-            return self._seq.first  # type: ignore
+            return self.seq().first  # type: ignore[union-attr]
         except AttributeError:
             return None
 
     @property
     def rest(self) -> "ISeq[T]":
-        self._realize()
         try:
-            return self._seq.rest  # type: ignore
+            return self.seq().rest  # type: ignore[union-attr]
         except AttributeError:
             return EMPTY
 
@@ -208,9 +260,9 @@ def sequence(s: Iterable) -> ISeq[Any]:
         return EMPTY
 
 
-def _seq_or_nil(s: ISeq) -> Optional[ISeq]:
+def _seq_or_nil(s: Optional[ISeq]) -> Optional[ISeq]:
     """Return None if a ISeq is empty, the ISeq otherwise."""
-    if s.is_empty:
+    if s is None or s.is_empty:
         return None
     return s
 
@@ -231,6 +283,12 @@ def _to_seq_iseq(o: ISeq) -> Optional[ISeq]:
     return _seq_or_nil(o)
 
 
+@to_seq.register(LazySeq)
+def _to_seq_lazyseq(o: LazySeq) -> Optional[ISeq]:
+    # Force evaluation of the LazySeq by calling o.seq() directly.
+    return o.seq()
+
+
 @to_seq.register(ISeqable)
 def _to_seq_iseqable(o: ISeqable) -> Optional[ISeq]:
     return _seq_or_nil(o.seq())

src/basilisp/lang/set.py

@@ -174,7 +174,9 @@ def disj(self, *elems: T) -> "PersistentSet[T]":
     def empty() -> "PersistentSet":
         return EMPTY
 
-    def seq(self) -> ISeq[T]:
+    def seq(self) -> Optional[ISeq[T]]:
+        if len(self._inner) == 0:
+            return None
         return sequence(self)
 
     def to_transient(self) -> TransientSet:

src/basilisp/lang/vector.py

@@ -165,7 +165,9 @@ def val_at(self, k, default=None):
     def empty() -> "PersistentVector[T]":
         return EMPTY
 
-    def seq(self) -> ISeq[T]:  # type: ignore[override]
+    def seq(self) -> Optional[ISeq[T]]:  # type: ignore[override]
+        if len(self._inner) == 0:
+            return None
         return sequence(self)
 
     def peek(self) -> Optional[T]: