From 320e2d22ef91b1d7ef194fb28c713839240b1457 Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Wed, 28 Apr 2021 20:26:59 -0700
Subject: lib: document gc problem related to seq-begin.

If seq-begin is used on an object that supports the iter-begin
method, there is a gc problem. This does not seem worth
fixing for the following reasons.

1. The seq-begin function is marked obsolescent. I removed
   its one and only internal use in the previous commit, so
   it won't be called unless application code uses it.

2. Objects supporting the iter-begin function are clearly
   developed as part of the new iteration protocol. It makes
   no sense to be newly developing such an object, along with
   new code which applies seq-begin to it.  There is likely
   zero code in the wild which uses either of these
   mechanisms.

* lib.c (seq_iter_get_oop, seq_iter_peek_oop,
seq_iter_get_fast_oop, seq_iter_peek_fast_oop): Add comments
documenting the issue.
---
 lib.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib.c b/lib.c
index f60500e6..95db82e4 100644
--- a/lib.c
+++ b/lib.c
@@ -571,6 +571,20 @@ static int seq_iter_get_oop(seq_iter_t *it, val *pval)
 {
   val iter = it->ui.iter;
 
+  /* The assignments to ui.iter and ui.next are wrong if the it structure is
+   * embedded inside a heap-allocated iterator object. The object could be a
+   * gen 1 object, whereas the value being assigned could be a gen 0.
+   *
+   * The only way this can happen is if the obsolescent seq-begin function is
+   * used on an object that supports the iter-begin method. The seq-begin
+   * constructor is the only function which creates a heap-allocated iterator
+   * which is initialized via seq_iter_init_with_info, which binds this
+   * seq_iter_get_oop function and its sisters.
+   *
+   * The other heap-allocated iterator type is iter-begin. iter-begin applies
+   * its own handling for OOP iterators; it doesn't set up seq_iter_t for
+   * objects of that type, and so these functions are not used.
+   */
   if (it->ul.next != nao) {
     val iter_step_meth = get_special_required_slot(iter, iter_step_m);
     *pval = it->ul.next;
@@ -597,6 +611,8 @@ static int seq_iter_peek_oop(seq_iter_t *it, val *pval)
 {
   val iter = it->ui.iter;
 
+  /* See comment in seq_iter_get_oop */
+
   if (it->ul.next != nao) {
     *pval = it->ul.next;
     return 1;
@@ -629,6 +645,8 @@ static int seq_iter_get_fast_oop(seq_iter_t *it, val *pval)
       *pval = item;
     }
 
+    /* See comment in seq_iter_get_oop */
+
     it->ui.iter = funcall1(iter_step_meth, iter);
     it->ul.next = nao;
     return 1;
@@ -646,6 +664,8 @@ static int seq_iter_peek_fast_oop(seq_iter_t *it, val *pval)
     return 1;
   }
 
+  /* See comment in seq_iter_get_oop */
+
   if (iter) {
     val iter_item_meth = get_special_required_slot(iter, iter_item_m);
     it->ul.next = *pval = funcall1(iter_item_meth, iter);
-- 
cgit v1.2.3