GNU bug report logs - #56347
Optimize/simplify STRING_SET_MULTIBYTE

Previous Next

Full log

Message #8 received at 56347 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 56347 <at> debbugs.gnu.org
Subject: Re: bug#56347: Optimize/simplify STRING_SET_MULTIBYTE
Date: Sat, 02 Jul 2022 09:17:06 +0300

> Date: Fri, 01 Jul 2022 19:32:05 -0400
> From:  Stefan Monnier via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The patch below simplifies code around STRING_SET_MULTIBYTE.
> Any objection?

Rationale?  Simplification in these cases is minimal, almost
non-existent, so it cannot be the only rationale.

> --- a/src/composite.c
> +++ b/src/composite.c
> @@ -1879,11 +1879,7 @@ Otherwise (for terminal display), FONT-OBJECT must be a terminal ID, a
>  	  for (i = SBYTES (string) - 1; i >= 0; i--)
>  	    if (!ASCII_CHAR_P (SREF (string, i)))
>  	      error ("Attempt to shape unibyte text");
> -	  /* 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?  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.

> --- a/src/lisp.h
> +++ b/src/lisp.h
> @@ -1637,12 +1637,10 @@ #define STRING_SET_UNIBYTE(STR)				\
>  
>  /* Mark STR as a multibyte string.  Assure that STR contains only
>     ASCII characters in advance.  */
> -#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?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.