bug#56347: Optimize/simplify STRING_SET_MULTIBYTE

[Eli Zaretskii <eliz@gnu.org>]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: 56347@debbugs.gnu.org
> Date: Sat, 02 Jul 2022 12:12:06 -0400
> 
> STRING_SET_MULTIBYTE is fundamentally evil because it changes the nature
> of an object.  Its current definition (like that of STRING_SET_UNIBYTE)
> is rather scary (it sometimes changes the nature of the arg passed to
> it, and sometimes replaces the arg with something else).

But do we have any alternatives?

> >> -	  /* STRING is a pure-ASCII string, so we can convert it (or,
> >> -	     rather, its copy) to multibyte and use that thereafter.  */
> >> -	  Lisp_Object string_copy = Fconcat (1, &string);
> >> -	  STRING_SET_MULTIBYTE (string_copy);
> >> -	  string = string_copy;
> >> +	  /* STRING is a pure-ASCII string, so we can treat it as multibyte.  */
> >
> > Did you actually try your change in the situations where this problem
> > pops up?
> 
> I don't even know how to go about doing that, no.

Make a character-composition rule that composes, say, two '-'
characters, and then display a buffer where you have adjacent dashes.

> > AFAIR, the code makes a copy of the string for good reasons:
> > the rest of handling of the string down the line barfs if we keep a
> > multibyte string here.
> 
> [ I assume you meant "barfs if we keep a *uni*byte string here".  ]

Yes.

> Where?

I don't remember, sorry.

> >> -#define STRING_SET_MULTIBYTE(STR)			\
> >> -  do {							\
> >> -    if (XSTRING (STR)->u.s.size == 0)			\
> >> -      (STR) = empty_multibyte_string;			\
> >> -    else						\
> >> -      XSTRING (STR)->u.s.size_byte = XSTRING (STR)->u.s.size; \
> >> +#define STRING_SET_MULTIBYTE(STR)			    \
> >> +  do {							    \
> >> +    eassert (XSTRING (STR)->u.s.size > 0);		    \
> >> +    XSTRING (STR)->u.s.size_byte = XSTRING (STR)->u.s.size; \
> >>    } while (false)
> >>  
> >>  /* Convenience functions for dealing with Lisp strings.  */
> >
> > You want to disallow uses of empty_multibyte_string? why?
> 
> No, I want to reduce the scope of semantics of the macro, e.g. so it can
> be implemented as a function rather than a macro and so it doesn't
> magically substitute empty_multibyte_string into a variable that held
> something else.

But the effect is that you disallow calling STRING_SET_MULTIBYTE on an
empty string, isn't it?