bug#41520: 28.0.50; Crash in character.h due to assertion error

[Pip Cet]

On Mon, May 25, 2020 at 7:30 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Mon, 25 May 2020 17:54:01 +0000
> > Cc: stefan@marxist.se, 41520@debbugs.gnu.org
> >
> > All I'm hoping for, at this point, is a "maybe, show me a patch".
>
> I don't know what to say, since it sounds like the "appetite comes
> with eating".  We never talked about PT_POS before.

You're correct. Sorry about that.

> Is the plan to make any popular position a struct?

The plan is to introduce additional struct-valued macros for things
like PT/PT_BYTE:

#define PT_POS POS (PT, PT_BYTE)

In particular, it's not an lvalue. That's important to me, since
assigning to PT_POS would be a severe bug.

> Like GPT, for example?

That's difficult. GPT is, of course, very special. I still think the
code is slightly more readable with an additional GPT_POS macro being
used where appropriate. There's a particular problem because using GPT
very often means modifying it, so maybe GPT_POS should be an lval.

> What about BEGV and ZV?

BEG, BEGV, ZV, and Z would all have _POS equivalents, and very often
using them results in more readable code.

> IOW, I don't understand the goal here.

There are multiple goals: I think this significantly aids readability,
and I think there might still be some minor bugs to catch, and future
bugs to avoid.

For debug builds only, it might make sense to include the object that
the bytepos-charpos relation is valid for, to catch cases where one
object's correspondence is used for another object.

> I think I did understand when we were talking about accessing characters by 
> buffer positions, and
> the bugs related to incorrect usage there, but now it sounds like the plot 
> thickens?

I hope that most code will follow a basic structure of being passed a
Lisp_Object or two (charpos/marker and object), converting that to a
pos_t, handing that to internal APIs, potentially receiving a pos_t
back and converting it back to a Lisp_Object, with only a few lines of
code deep down the call stacks actually unwrapping the pos_t and
manipulating it directly. That means there are a few more cases than
accessing buffer text: comparing two positions, for example, walking a
buffer or string by incrementing or decrementing them, adding two
positions or subtracting them.

(It's true that all kinds of crazy experiments would be easier with
code that follows this structure, but that's a side effect: the goal
really is to increase readability a little in a lot of places.)

Take, for example, this code:

    ch = bidi_fetch_char (cpos += nc, bpos += clen, &disp_pos, &dpp, &bs,
                  bidi_it->w, fwp, &clen, &nc)

"nc" and "clen" belong together, and so do cpos and bpos. I find the
names don't make that very obvious, and simply reducing the number of
arguments bidi_fetch_char takes by two helps a little.