Re: Questionable code in handling of wordend in the regexp engine in regex-emacs.c

[Alan Mackenzie <acm@muc.de>]

Hello, Eli.

On Mon, Mar 04, 2019 at 19:25:57 +0200, Eli Zaretskii wrote:
> > Date: Sat, 2 Mar 2019 13:18:01 +0000
> > Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > Instead I moved the eassert to after the bit where it checks for unibyte
> > buffers, giving this:



> > diff --git a/src/marker.c b/src/marker.c
> > index b58051a8c2..0b2e1bf5c6 100644
> > --- a/src/marker.c
> > +++ b/src/marker.c
> > @@ -332,6 +332,10 @@ buf_bytepos_to_charpos (struct buffer *b, ptrdiff_t bytepos)
> >    if (best_above == best_above_byte)
> >      return bytepos;
> > 
> > +  /* Check bytepos is not in the middle of a character. */
> > +  eassert (bytepos >= BUF_Z_BYTE (b)
> > +           || CHAR_HEAD_P (BUF_FETCH_BYTE (b, bytepos)));
> > +
> >    best_below = BEG;
> >    best_below_byte = BEG_BYTE;

[ .... ]

> I was forced to disable this assertion for now: I bootstrapped today a
> clean checkout, and several jobs that run during the bootstrap
> triggered the assertion.  It turns out there's one legitimate use case
> when bytepos _can_ be in the middle of a multibyte sequence: when we
> convert a buffer from unibyte to multibyte.  There are comments to
> that effect in set_intervals_multibyte_1.

> I see 2 possible ways to handle this: (1) remove the assertion for
> good, or (2) change buf_bytepos_to_charpos to accept one more
> argument, telling it whether to make this check, and then modify all
> the callers except those in set_intervals_multibyte_1 to pass 'true'
> as that argument.

> Thoughts?

First of all, sorry I wasn't here yesterday to deal with it.

I don't think I like alternative (2) - it's ugly, and how much do we
really need this eassert anyway?  It's turned out not to be such a good
idea after all.  I would favour alternative (1), just removing the thing
altogether.

-- 
Alan Mackenzie (Nuremberg, Germany).