Re: Edebug corrupting point in buffers; we need buffer-point and set-buffer-point, perhaps.

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Mon, Oct 31, 2022 at 16:50:52 +0200, Eli Zaretskii wrote:
> > Date: Mon, 31 Oct 2022 14:32:12 +0000
> > Cc: emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Anyhow, I proposed buffer-point and set-buffer-point.  They would be a
> > lot faster than set-buffer followed by point and goto-char.  Here is my
> > first version of these.  What do you think?

> I'm not sure performance in a debugger is a reason good enough to add
> 2 more primitives.  The fact that we didn't need them until now should
> tell us something, no?

Well, I timed it.  With 207 buffers, creating an alist of (buffer .
buffere-point) with my new function was 17 times as fast as using
with-current-buffer and point.  Restoring the buffer-points was 8 times
as fast with my new function.  It's probably moot, though, since the
"slow" restoration only took 0.00137 seconds for all 207 buffers.

But on the other hand, these two functions feel like they ought to exist.
They could save a lot of clumsy programming with swapping the buffer,
just to get or set point.

> Stefan, Lars, WDYT?

> Anyway, a couple of minor comments:

> > +DEFUN ("buffer-point", Fbuffer_point, Sbuffer_point, 1, 1, 0,
> > +       doc: /* Return the buffer point of BUFFER-OR-NAME.
> > +The argument may be a buffer or the name of an existing buffer.  */)
> > +  (Lisp_Object buffer_or_name)

> Why not an optional argument to 'point'?  And why in buffer.c and not
> in editfns.c?

I'm not sure what you mean by an optional argument, here.

buffer.c was the first file that sprang to mind.  It could easily be
somewhere else, though.

> > +  return (make_fixnum (b->pt));

> Please never-ever use b->pt etc. directly.  We have BUF_PT and other
> macros for that, and for a good reason.

BUF_PT and friends work specifically on current_buffer.  The whole idea
of the new functions is to avoid having to switch buffers.

> > +  CHECK_FIXNUM_COERCE_MARKER (pos);
> > +  p = XFIXNUM (pos);

> This is sub-optimal: a marker holds both character and byte position,
> and you lose it here.  Ignoring the byte position is only justified if
> the marker belongs to the wrong buffer.

And I need to check the marker's in the correct buffer, too.  But thanks
for the tip!

> > +  if (p < b->begv) p = b->begv;
> > +  if (p > b->zv) p = b->zv;

> We have clip_to_bounds.  And again, always use BUF_BEGV and BUF_ZV,
> not literal references to members of struct buffer.

I knew there was some sort of function, but couldn't think of the name,
and couldn't find a way to search for it.  ;-(

> > +  SET_PT (p);

> We have SET_BUF_PT_BOTH.

OK.  That SET_PT was a mistake; it sets point in the current buffer, not
the buffer that was the argument.  But I take the point (no pun
intended).  If we've already got point_byte, there's no point calculating
it all over again.

Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).