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

[Pip Cet]

On Mon, May 25, 2020 at 4:09 PM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Pip Cet <pipcet@gmail.com>
> > Date: Mon, 25 May 2020 15:16:09 +0000
> > Cc: stefan@marxist.se, 41520@debbugs.gnu.org
> >
> > > But wouldn't it be strange to see a macro that accepts a struct, but
> > > only uses one member of that struct?
> >
> > I don't think so. CHARPOS and BYTEPOS already exist, and that's
> > precisely what they do.
> >
> > What is a little strange is that the ancient convention of not
> > returning struct types is still followed in much of Emacs.
>
> It's more expensive.

Only for very large structs, or on old architectures.

> That's what I meant when I said "strange": why
> would we fill 2 fields of a struct, but use only one?

As I said, I'm not talking about cases in which one variable suffices.
It's those cases where we have:

ptrdiff_t charpos;
ptrdiff_t bytepos;

(not usually named like that, or indeed consistently).

My suggestion is we use

pos_t pos;

and then pos.charpos and pos.bytepos as appropriate.

> > > I mean we already have assertions: that's what eassume does in a debug
> > > build.
> >
> > Yes, but we could do with some stricter checking, I think.
>
> It cannot catch the cases where we put a character position into the
> byte position slot.  That's the general problem with using simple
> scalars.

Incorrect code becomes way more obvious.

bytepos = PT;

is incorrect but shorter than the correct version.

pos.bytepos = PT_POS.charpos;
pos.charpos = PT_POS.bytepos;

is much more obviously wrong, and the correct version is simply:

pos =  PT_POS;

On non-obsolescent architectures, returning a two-word struct is
cheaper than accessing two parameters through pointers, too; and
that's only relevant for those few cases in which the function isn't
inlined, anyway.

All I'm hoping for, at this point, is a "maybe, show me a patch".