bug#60058: 29.0.60; overlay-lists does not conform to its docstring

bug#60058: 29.0.60; overlay-lists does not conform to its docstring

[Kai Ma]

The docstring of overlay-lists states that the return value is a cons
pair, and the CDR is not always nil.  However, the code does not do what
it claims to do.

    DEFUN ("overlay-lists", Foverlay_lists, Soverlay_lists, 0, 0, 0,
           doc: /* Return a pair of lists giving all the overlays of the current buffer.
    The car has all the overlays before the overlay center;
    the cdr has all the overlays after the overlay center.
    Recentering overlays moves overlays between these lists.
    The lists you get are copies, so that changing them has no effect.
    However, the overlays you get are the real objects that the buffer uses. */)
      (void)
    {
      Lisp_Object overlays = Qnil;
      struct itree_node *node;
    
      ITREE_FOREACH (node, current_buffer->overlays, BEG, Z, DESCENDING)
        overlays = Fcons (node->data, overlays);
    
      return Fcons (overlays, Qnil);
    }

This change in behavior has led to at least one breakage in the wild:
symbol-overlay cannot count overlays correctly.

I think there is a problem either in the code or in the docstring.

	Kai

bug#60058: 29.0.60; overlay-lists does not conform to its docstring

[Eli Zaretskii]

> From: Kai Ma <justksqsf@gmail.com>
> Date: Wed, 14 Dec 2022 13:22:07 +0800
> 
> The docstring of overlay-lists states that the return value is a cons
> pair, and the CDR is not always nil.  However, the code does not do what
> it claims to do.
> 
>     DEFUN ("overlay-lists", Foverlay_lists, Soverlay_lists, 0, 0, 0,
>            doc: /* Return a pair of lists giving all the overlays of the current buffer.
>     The car has all the overlays before the overlay center;
>     the cdr has all the overlays after the overlay center.
>     Recentering overlays moves overlays between these lists.
>     The lists you get are copies, so that changing them has no effect.
>     However, the overlays you get are the real objects that the buffer uses. */)
>       (void)
>     {
>       Lisp_Object overlays = Qnil;
>       struct itree_node *node;
>     
>       ITREE_FOREACH (node, current_buffer->overlays, BEG, Z, DESCENDING)
>         overlays = Fcons (node->data, overlays);
>     
>       return Fcons (overlays, Qnil);
>     }
> 
> This change in behavior has led to at least one breakage in the wild:
> symbol-overlay cannot count overlays correctly.
> 
> I think there is a problem either in the code or in the docstring.

The problem is not in overlay-lists, IMO, the problem (if there is
one) is in overlay-recenter: it is a no-op in Emacs 29 and later.

I think we have two alternatives:

  . reimplement overlay-recenter for the new overlay representation
  . document that overlay-recenter is a no-op, as an incompatible Lisp
    change, and recommend that Lisp program simply stop relying on its
    effect in Emacs >= 29: after all, the sole purpose of that
    function was to speed up overlay access, and now this is not needed

I tend to do the latter.  Lars, Stefan, any ideas or comments?

bug#60058: 29.0.60; overlay-lists does not conform to its docstring

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> The problem is not in overlay-lists, IMO, the problem (if there is
> one) is in overlay-recenter: it is a no-op in Emacs 29 and later.

The use of `overlay-recenter` together with `overlay-lists` took me
by surprise.  So yes, I think we should document that `overlay-recenter`
is a no-op.  We should also mark it obsolete.

The code for `symbol-overlay-get-list` should likely use
`overlays-in` instead.


        Stefan

bug#60058: 29.0.60; overlay-lists does not conform to its docstring

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Kai Ma <justksqsf@gmail.com>,  Lars Ingebrigtsen <larsi@gnus.org>,
>   60058@debbugs.gnu.org
> Date: Wed, 14 Dec 2022 09:21:44 -0500
> 
> > The problem is not in overlay-lists, IMO, the problem (if there is
> > one) is in overlay-recenter: it is a no-op in Emacs 29 and later.
> 
> The use of `overlay-recenter` together with `overlay-lists` took me
> by surprise.  So yes, I think we should document that `overlay-recenter`
> is a no-op.  We should also mark it obsolete.

I didn't mark it obsolete yet.  Having Emacs 29.1 annoy programs that
call a function that is a no-op, and adding that so close to the
release, sounds like not the best idea.  Maybe we should consider
obsoleting it in Emacs 30.

Otherwise, I did update the documentation to be consistent with the
new implementation of the overlays (see also bug#59996), and I'm
therefore closing this bug report.