Bug with src/composite.c in emacs 27 ?

empty_string_issue.org

empty_string_issue

Issue

While trying to use elisp code to get ligatures working on a build of emacs-27 branch, I noticed that a few ligatures from the table was freezing emacs.

Recipe

Emacs version

I think you only need an emacs-27 build --with-harfbuzz

The version is 27.0.60 with harfbuzz feature.

I think I was on commit 5841240 but I’m not 100% positive since I don’t have the machine easily available. I know I pulled emacs-27 between Fri, 10 Jan 2020 08:00:00 GMT and Fri, 10 Jan 2020 11:00:00 GMT

Note : I also had the issue in 26.3 (without harfbuzz) trying the code to see if I can reproduce

Set up a buffer

This is extracted from microsoft/cascadia-code#153 on Github Link to the issue

(defvar composition-ligature-table (make-char-table nil))
(require 'composite)

(let ((alist
       '(
         (?* . ".\\(?:\\(\\*\\*\\|[*>]\\)[*>]?\\)")
         )))
  (dolist (char-regexp alist)
    (set-char-table-range composition-ligature-table (car char-regexp)
                          `([,(cdr char-regexp) 0 font-shape-gstring]))))

(set-char-table-parent composition-ligature-table composition-function-table)

(setq-local composition-function-table composition-ligature-table)

Test fonts

• Write a lot of * : *****
• Delete asterisks one by one (DEL)
• EXPECTED : asterisks get deleted one by one

With ligatures

Fira Code Github repo has a ligature for *** so everything works normally.

Without ligatures

Using Fira Mono for example (or any font which does not have ligatures for ***), you should encounter the same issue as me, which is emacs stuttering/being unresponsive (I have to send USR2 to get control back)

Possible solution

A possible solution would be “not” to add entries which don’t return ligatures for a given font ; but I think it would be easier if we could add a lot of common prog-mode ligatures and leave that code somewhere so any user can pull the same code and have as many ligatures as their font allows.

This reduces the maintenance burden if we don’t have to maintain an alist for each stylistic set of each font.

It could even be possible to only set an alist per major-mode, so that --> does not do anything for languages where we don’t expect ligatures.

Patch for the other solution

With this patch I was able to stop having the infinite freeze/stuttering I think the issue is that :

• when the font does not have a ligature for the matching pattern
• it somehow passes an empty string =”“= in this if-statement
• (if "" 'not-nil) return =’not-nil= so it goes to the else branch
• STRING_MULTIBYTE("") is false because an empty string doesn’t have the marker
• composite errors and loops

diff --git a/src/composite.c b/src/composite.c
index 53e6930b5f..1151721d61 100644
--- a/src/composite.c
+++ b/src/composite.c
@@ -1735,7 +1735,7 @@ Otherwise (for terminal display), FONT-OBJECT must be a terminal ID, a
   if (NILP (string))
     {
       if (NILP (BVAR (current_buffer, enable_multibyte_characters)))
-       error ("Attempt to shape unibyte text");
+       error ("Attempt to shape unibyte text \"%s\" in non multibyte buffer", string);
       validate_region (&from, &to);
       frompos = XFIXNAT (from);
       topos = XFIXNAT (to);
@@ -1745,8 +1745,8 @@ Otherwise (for terminal display), FONT-OBJECT must be a terminal ID, a
     {
       CHECK_STRING (string);
       validate_subarray (string, from, to, SCHARS (string), &frompos, &topos);
-      if (! STRING_MULTIBYTE (string))
-       error ("Attempt to shape unibyte text");
+      if (strlen(string) != 0 && ! STRING_MULTIBYTE (string))
+       error ("Attempt to shape unibyte text \"%s\"", string);
       frombyte = string_char_to_byte (string, frompos);
     }