Question about composite.c

Question about composite.c

[Gerry Agbobada]

#+TITLE: empty_string_issue

The rest of this file has been edited with org-mode, to allow simple tangling to
create an elisp file to try to setup the bug I had ; and have
highlighting for the
diff I tried.

* 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.

I wasn't able to reproduce properly using =emacs -q=, but I was able to
reproduce it in 2 different major modes using a heavily customized emacs (Doom
Emacs). I was able to patch emacs C code to make my issue stop though; so I
mainly want to know if that is a logic error I was able to find out or just
something I should keep investigating on the elisp side.

* Recipe
** Emacs version
I'm playing with emacs-27 builds =--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.

The =src/composite.c= file I was able to single out didn't change since then it
seems.

** Set up a buffer
This is extracted from microsoft/cascadia-code#153 on Github
[[https://github.com/microsoft/cascadia-code/issues/153][Link to the issue]]
#+BEGIN_SRC emacs-lisp :tangle yes
(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)
#+END_SRC

** Test fonts
- Write a lot of * : *****
- Delete asterisks one by one (=DEL=)
- EXPECTED : asterisks get deleted one by one
- ACTUAL : emacs freezes and after modifying the =src/composite.c= file I found
  out the error is =Attempt to shape unibyte text= from this
[[https://github.com/emacs-mirror/emacs/blob/b651939aaf06f4f1ddcd1f750bb8825546027b96/src/composite.c#L1749][if
branch]] with an
  *empty string*.

I think my error may come from having a composition-table where a replacement
triggered by =prettify-symbols= occurs before the regex for
=composition-ligature-table= happens, so there's only an empty string for
replacement and there's an error because it doesn't pass the test.

* Patch to "make it work"
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

Or maybe the error comes from having a composition-table where a replacement
triggered by =prettify-symbols= occurs before the regex for
=composition-ligature-table= happens, so there's only an empty string for
replacement and there's an error because it doesn't pass the test.

NOTE : I know this is a "dirty" patch from thw warning I got from the compiler
(i.e. using strlen on the =string= variable here is not clean.) but I feel it
conveys what I mean in a better way, and to be honest I don't know what the
clean version is.

NOTE 2 : this patch also shows how I was able to find out which if-branch
triggered. I guess the reason for not displaying the bad string is to avoid
side-effects in the minibuffer.

#+BEGIN_SRC diff :tangle no
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);
     }
#+END_SRC

* Question
I guess the only question is : what's supposed to happen when =string= is an
empty lisp string in this condition ?

Gerry AGBOBADA

Re: Question about composite.c

[Eli Zaretskii]

> From: Gerry Agbobada <gagbobada@gmail.com>
> Date: Mon, 20 Jan 2020 23:17:19 +0100
> 
> (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)

Can you tell what kind of ligatures you wanted to support that
required such a non-trivial regexp?

Also, why did you need to use a separate char-table instead of
composition-function-table?

> I think my error may come from having a composition-table where a replacement
> triggered by =prettify-symbols= occurs before the regex for
> =composition-ligature-table= happens, so there's only an empty string for
> replacement and there's an error because it doesn't pass the test.

It's not just an empty string, it's a _unibyte_ string.  How did that
happen?

More importantly, please don't use prettify-symbols-mode, which are
based on static compositions, together with automatic compositions.
Static compositions are an obsolete feature, it lacks support for some
modern Emacs features (e.g., bidirectional text), and we should remove
it from Emacs at some future point -- but not before we implement a
replacement for it using composition-function-table and related
machinery.  Mixing these two incompatible compositions is asking for
trouble.  If you turn off prettify-symbols-mode, does the problem go
away?

> 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);

This cannot be right.  We cannot meaningfully compose unibyte text,
because it is not made of characters, it is made of raw bytes, and
therefore you cannot meaningfully reference composition-function-table
by such raw bytes.  The errors are correct, and must stay that way.
You need to debug this further to understand how come you ended up in
this condition, and then fix whatever root cause caused that.

> * Question
> I guess the only question is : what's supposed to happen when =string= is an
> empty lisp string in this condition ?

There's no problem with empty strings here, as long as they are
multibyte.

Re: Question about composite.c

[Gerry Agbobada]

> Can you tell what kind of ligatures you wanted to support that
> required such a non-trivial regexp?

This was mainly to set the composition table once in my .el files and be
able to change fonts and stylistic sets without having to update the table.

Therefore, the main goal was to "catch common programming ligatures
starting with ?*"

> Also, why did you need to use a separate char-table instead of
> composition-function-table?

I used another char-table so the variable is easily accessible to modify
parts of it. Then again, maybe a poor design decision, but just a design
decision.

> It's not just an empty string, it's a _unibyte_ string.  How did that
> happen?

That's the main part I was trying to understand, since I could not
reproduce it in an easy way, I was wondering if an empty string can be
unibyte or multibyte. I have my answer now :)

> More importantly, please don't use prettify-symbols-mode, which are
> based on static compositions, together with automatic compositions.
> Static compositions are an obsolete feature, it lacks support for some
> modern Emacs features (e.g., bidirectional text), and we should remove
> it from Emacs at some future point -- but not before we implement a
> replacement for it using composition-function-table and related
> machinery.  Mixing these two incompatible compositions is asking for
> trouble.  If you turn off prettify-symbols-mode, does the problem go
> away?

I'll test it (I don't have a build to test it currently), but I suspect that it
would stop. I'll send an update if you're interested, but I don't think
this was my main question so I'll leave that unanswered if you think
the answer is not important.

> This cannot be right.  We cannot meaningfully compose unibyte text,
> because it is not made of characters, it is made of raw bytes, and
> therefore you cannot meaningfully reference composition-function-table
> by such raw bytes.  The errors are correct, and must stay that way.
> You need to debug this further to understand how come you ended up in
> this condition, and then fix whatever root cause caused that.
> [...]
> There's no problem with empty strings here, as long as they are
> multibyte.

That the main point I don't understand : is there a way to mark an empty
string as a multibyte string ? I found out about Lisp_String.size_byte but I
didn't find the code where strings were transformed into Lisp_String objects.

At least now I know I can continue investigating on the elisp side, thanks for
your answers !

Gerry

Re: Question about composite.c

[Eli Zaretskii]

> From: Gerry Agbobada <gagbobada@gmail.com>
> Date: Tue, 21 Jan 2020 19:57:07 +0100
> Cc: emacs-devel@gnu.org
> 
> Therefore, the main goal was to "catch common programming ligatures
> starting with ?*"

Can you show examples of those, if not their exhaustive list?

> > Also, why did you need to use a separate char-table instead of
> > composition-function-table?
> 
> I used another char-table so the variable is easily accessible to modify
> parts of it. Then again, maybe a poor design decision, but just a design
> decision.

I don't see why it would be easier to access/modify a separate table.

> > It's not just an empty string, it's a _unibyte_ string.  How did that
> > happen?
> 
> That's the main part I was trying to understand, since I could not
> reproduce it in an easy way, I was wondering if an empty string can be
> unibyte or multibyte. I have my answer now :)

If you post a complete reproduction recipe, starting from "emacs -Q",
perhaps I will be able to help you understand what is the immediate
cause of the problem.

> > There's no problem with empty strings here, as long as they are
> > multibyte.
> 
> That the main point I don't understand : is there a way to mark an empty
> string as a multibyte string ?

We could look into that, but why would an empty string end up in
composition-get-gstring anyway? there's no meaningful way of composing
such a string.

> At least now I know I can continue investigating on the elisp side, thanks for
> your answers !

I think the important part is to understand why Emacs is trying to
compose an empty string.

Re: Question about composite.c

[Gerry Agbobada]

> Can you show examples of those, if not their exhaustive list?

Between iosevka, Fira Code and Cascadia Code, ligatures that may
happen starting with * are :
- *
- **
- ***
- */
- *>
- *)

iosevka : https://raw.githubusercontent.com/be5invis/Iosevka/master/images/ligations.png
FiraCode : https://github.com/tonsky/FiraCode/blob/master/showcases/all_ligatures.png
Cascadia : https://github.com/microsoft/cascadia-code/wiki/Coding-ligature-coverage

> I don't see why it would be easier to access/modify a separate table.

Fair enough, I'll probably remove the indeirection once I find out
my elisp issue

> If you post a complete reproduction recipe, starting from "emacs -Q",
> perhaps I will be able to help you understand what is the immediate
> cause of the problem.
> [...]
> I think the important part is to understand why Emacs is trying to
> compose an empty string.

I'm trying to do this to understand better yes. In my "bloated" emacs
setup, I still have the "trying to compose an empty string" issue without
prettify-symbols-mode, so I'll just continue playing with my short .el file
until I can find a way to properly reproduce.

Gerry

Re: Question about composite.c

[Gerry Agbobada]

Hello,

It took me a good while, but I found a case where :
- I can reproduce with =emacs -q=  (Reproducing infinite looping in
composition_get_gstring with an empty non-nil string)
- prettify-symbols-mode is not enabled

The example includes the code of eros.el package, which creates an
overlay when calling eval-last-sexp. The overlay is supposed to start
with `=>`, and when I have this in my composition-function-table it
triggers the loop.
As explained in the file linked below, with the cursor on `(+ 3 2)|`
I'd expect `C-x C-e` to create an overlay with `=> 5`. Instead, it
loops indefinitely and the only way out is to kill -9 emacs (kill
-USR2 doesn't help)

Question is : is there something I should do from elisp point of view
(as a user of eros, and composition-function-table) to avoid this
problem ? I'm still not sure about the code path that I'm following to
lead to this point. :(

I'm writing this from gmail, I wasn't able to paste the file in the
editor respecting the newlines, so I pushed it in a gist... I'd prefer
the code to be archived in the mailing list, but until I find a better
mail client than gmail online one, I won't be able to do it sorry.

For the time being, it is here
https://gist.github.com/gagbo/13239f12cfd6b90cce32b1c255f2553d (325
lines long)
Feel free to paste it in the conversation if you feel it is safer.
Someday I'll stop using Gmail I suppose.

Best regards
Gerry

Le mer. 22 janv. 2020 à 09:55, Gerry Agbobada <gagbobada@gmail.com> a écrit :
>
> > Can you show examples of those, if not their exhaustive list?
>
> Between iosevka, Fira Code and Cascadia Code, ligatures that may
> happen starting with * are :
> - *
> - **
> - ***
> - */
> - *>
> - *)
>
> iosevka : https://raw.githubusercontent.com/be5invis/Iosevka/master/images/ligations.png
> FiraCode : https://github.com/tonsky/FiraCode/blob/master/showcases/all_ligatures.png
> Cascadia : https://github.com/microsoft/cascadia-code/wiki/Coding-ligature-coverage
>
> > I don't see why it would be easier to access/modify a separate table.
>
> Fair enough, I'll probably remove the indeirection once I find out
> my elisp issue
>
> > If you post a complete reproduction recipe, starting from "emacs -Q",
> > perhaps I will be able to help you understand what is the immediate
> > cause of the problem.
> > [...]
> > I think the important part is to understand why Emacs is trying to
> > compose an empty string.
>
> I'm trying to do this to understand better yes. In my "bloated" emacs
> setup, I still have the "trying to compose an empty string" issue without
> prettify-symbols-mode, so I'll just continue playing with my short .el file
> until I can find a way to properly reproduce.
>
> Gerry

Re: Question about composite.c

[Noam Postavsky]

On Tue, 14 Apr 2020 at 18:44, Gerry Agbobada <gagbobada@gmail.com> wrote:

> I'm writing this from gmail, I wasn't able to paste the file in the
> editor respecting the newlines, so I pushed it in a gist... I'd prefer
> the code to be archived in the mailing list,

FYI, posting code as an attachment generally works okay for that.

Re: Question about composite.c

[Gerry Agbobada]

[-- Attachment #1: Type: text/plain, Size: 479 bytes --]

Oh, sorry, I should have done that.
Gerry

Le mer. 15 avr. 2020 à 03:10, Noam Postavsky <npostavs@gmail.com> a écrit :
>
> On Tue, 14 Apr 2020 at 18:44, Gerry Agbobada <gagbobada@gmail.com> wrote:
>
> > I'm writing this from gmail, I wasn't able to paste the file in the
> > editor respecting the newlines, so I pushed it in a gist... I'd prefer
> > the code to be archived in the mailing list,
>
> FYI, posting code as an attachment generally works okay for that.

[-- Attachment #2: ligature-bug.el --]
[-- Type: text/x-emacs-lisp, Size: 11500 bytes --]

;;; ligature-bug.el --- Reproduce an autocomposition bug -*- lexical-binding: t; -*-
;;; Commentary:
;;
;;  Reproducer for the composition table bug
;;
;;;; Method
;;
;; Evaluate the file, and then use eros + eval-last-sexp on the addition
;; Expected : "=> 5" is displayed at cursor, with a shiny ligature
;; Actual : inifinite looping, trying to "shape unibyte text",
;;          which happens to be an empty string (from my earlier debugging)
;;
;;
;;;; Commit information :
;; commit e1f0e0892232221e6333b24788b97942c83ec738 (HEAD -> emacs-27, origin/emacs-27)
;; Author: Eli Zaretskii <eliz@gnu.org>
;; Date:   Wed Mar 25 21:54:21 2020 +0200
;;
;; * lisp/files.el (directory-files-recursively): Doc fix.  (Bug#40202)
;;
;;
;;; Code:

;;;; Include eros
;; This is the smallest package I found with which I can reproduce my issue
;; I paste the source here for convenience, commenting only the provide call
;; and adding 2 levels to section headings (so they are under "include eros")

;;;;; eros.el --- Evaluation Result OverlayS for Emacs Lisp   -*- lexical-binding: t; -*-

;; Copyright (C) 2016-2018  Tianxiang Xiong

;; Author: Tianxiang Xiong <tianxiang.xiong@gmail.com>
;; Keywords: convenience, lisp
;; Package-Requires: ((emacs "24.4"))
;; URL: https://github.com/xiongtx/eros
;; Version: 0.1.0

;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program.  If not, see <http://www.gnu.org/licenses/>.

;;;;; Commentary:

;; Evaluation result overlays for Emacs Lisp.

;; The code is mostly taken from CIDER.  For more about CIDER, see:
;; https://github.com/clojure-emacs/cider

;;;;; Code:

(require 'cl-lib)

\f
;; Customize

(defgroup eros nil
  "Evaluation Result OverlayS for Emacs Lisp"
  :prefix "eros-"
  :group 'lisp)

(defcustom eros-eval-result-prefix "=> "
  "The prefix displayed in the minibuffer before a result value."
  :group 'eros
  :type 'string
  :package-version '(eros "0.1.0"))

(defface eros-result-overlay-face
  '((((class color) (background light))
     :background "grey90" :box (:line-width -1 :color "yellow"))
    (((class color) (background dark))
     :background "grey10" :box (:line-width -1 :color "black")))
  "Face used to display evaluation results at the end of line.
If `eros-overlays-use-font-lock' is non-nil, this face is applied
with lower priority than the syntax highlighting."
  :group 'eros
  :package-version '(eros "0.1.0"))

(defcustom eros-overlays-use-font-lock t
  "If non-nil, results overlays are font-locked as Clojure code.
If nil, apply `eros-result-overlay-face' to the entire overlay instead of
font-locking it."
  :group 'eros
  :type 'boolean
  :package-version '(eros "0.1.0"))

(defcustom eros-eval-result-duration 'command
  "Duration, in seconds, of eval-result overlays.

If nil, overlays last indefinitely.

If the symbol `command', they're erased before the next command."
  :group 'eros
  :type '(choice (integer :tag "Duration in seconds")
                 (const :tag "Until next command" command)
                 (const :tag "Last indefinitely" nil))
  :package-version '(eros "0.1.0"))

\f
;; Overlay

(defun eros--make-overlay (l r type &rest props)
  "Place an overlay between L and R and return it.

TYPE is a symbol put on the overlay's category property.  It is
used to easily remove all overlays from a region with:

    (remove-overlays start end 'category TYPE)

PROPS is a plist of properties and values to add to the overlay."
  (let ((o (make-overlay l (or r l) (current-buffer))))
    (overlay-put o 'category type)
    (overlay-put o 'eros-temporary t)
    (while props (overlay-put o (pop props) (pop props)))
    (push #'eros--delete-overlay (overlay-get o 'modification-hooks))
    o))

(defun eros--delete-overlay (ov &rest _)
  "Safely delete overlay OV.

Never throws errors, and can be used in an overlay's
modification-hooks."
  (ignore-errors (delete-overlay ov)))

(cl-defun eros--make-result-overlay (value &rest props &key where duration (type 'result)
                                           (format (concat " " eros-eval-result-prefix "%s "))
                                           (prepend-face 'eros-result-overlay-face)
                                           &allow-other-keys)
  "Place an overlay displaying VALUE at the end of line.

VALUE is used as the overlay's after-string property, meaning it
is displayed at the end of the overlay.  The overlay itself is
placed from beginning to end of current line.

Return nil if the overlay was not placed or if it might not be
visible, and return the overlay otherwise.

Return the overlay if it was placed successfully, and nil if it
failed.

This function takes some optional keyword arguments:

- If WHERE is a number or a marker, apply the overlay over the
  entire line at that place (defaulting to `point').  If it is a
  cons cell, the car and cdr determine the start and end of the
  overlay.

- DURATION takes the same possible values as the
  `eros-eval-result-duration' variable.

- TYPE is passed to `eros--make-overlay' (defaults to `result').

- FORMAT is a string passed to `format'.  It should have exactly
  one %s construct (for VALUE).

All arguments beyond these (PROPS) are properties to be used on
the overlay."
  (declare (indent 1))
  (while (keywordp (car props))
    (setq props (cddr props)))
  ;; If the marker points to a dead buffer, don't do anything.
  (let ((buffer (cond
                 ((markerp where) (marker-buffer where))
                 ((markerp (car-safe where)) (marker-buffer (car where)))
                 (t (current-buffer)))))
    (with-current-buffer buffer
      (save-excursion
        (when (number-or-marker-p where)
          (goto-char where))
        ;; Make sure the overlay is actually at the end of the sexp.
        (skip-chars-backward "\r\n[:blank:]")
        (let* ((beg (if (consp where)
                        (car where)
                      (save-excursion
                        (backward-sexp 1)
                        (point))))
               (end (if (consp where)
                        (cdr where)
                      (line-end-position)))
               (display-string (format format value))
               (o nil))
          (remove-overlays beg end 'category type)
          (funcall (if eros-overlays-use-font-lock
                       #'font-lock-prepend-text-property
                     #'put-text-property)
                   0 (length display-string)
                   'face prepend-face
                   display-string)
          ;; If the display spans multiple lines or is very long, display it at
          ;; the beginning of the next line.
          (when (or (string-match "\n." display-string)
                    (> (string-width display-string)
                       (- (window-width) (current-column))))
            (setq display-string (concat " \n" display-string)))
          ;; Put the cursor property only once we're done manipulating the
          ;; string, since we want it to be at the first char.
          (put-text-property 0 1 'cursor 0 display-string)
          (when (> (string-width display-string) (* 3 (window-width)))
            (setq display-string
                  (concat (substring display-string 0 (* 3 (window-width)))
                          "...\nResult truncated.")))
          ;; Create the result overlay.
          (setq o (apply #'eros--make-overlay
                         beg end type
                         'after-string display-string
                         props))
          (pcase duration
            ((pred numberp) (run-at-time duration nil #'eros--delete-overlay o))
            (`command (if this-command
                          (add-hook 'pre-command-hook
                                    #'eros--remove-result-overlay
                                    nil 'local)
                        (eros--remove-result-overlay))))
          (let ((win (get-buffer-window buffer)))
            ;; Left edge is visible.
            (when (and win
                       (<= (window-start win) (point))
                       ;; In 24.3 `<=' is still a binary predicate.
                       (<= (point) (window-end win))
                       ;; Right edge is visible. This is a little conservative
                       ;; if the overlay contains line breaks.
                       (or (< (+ (current-column) (string-width value))
                              (window-width win))
                           (not truncate-lines)))
              o)))))))

(defun eros--remove-result-overlay ()
  "Remove result overlay from current buffer.

This function also removes itself from `pre-command-hook'."
  (remove-hook 'pre-command-hook #'eros--remove-result-overlay 'local)
  (remove-overlays nil nil 'category 'result))

(defun eros--eval-overlay (value point)
  "Make overlay for VALUE at POINT."
  (eros--make-result-overlay (format "%S" value)
    :where point
    :duration eros-eval-result-duration)
  value)

\f
;; API

(defun eros-eval-last-sexp (eval-last-sexp-arg-internal)
  "Wrapper for `eval-last-sexp' that overlays results."
  (interactive "P")
  (eros--eval-overlay
   (eval-last-sexp eval-last-sexp-arg-internal)
   (point)))

(defun eros-eval-defun (edebug-it)
  "Wrapper for `eval-defun' that overlays results."
  (interactive "P")
  (eros--eval-overlay
   (eval-defun edebug-it)
   (save-excursion
     (end-of-defun)
     (point))))

\f
;; Minor mode

;;;###autoload
(define-minor-mode eros-mode
  "Display Emacs Lisp evaluation results overlays."
  :global t
  (if eros-mode
      (progn
        (global-set-key [remap eval-last-sexp] #'eros-eval-last-sexp)
        (global-set-key [remap eval-defun] #'eros-eval-defun))
    (global-set-key [remap eval-last-sexp] nil)
    (global-set-key [remap eval-defun] nil)))

\f
;; (provide 'eros)
;;;;; eros.el ends here

;;;; Enable eros
;; It writes an overlay with the result of an evaluation on eval-last-sexp.
;; The overlay is (concat "=> " result) basically
(eros-mode 1)

(defvar other-composition-ligature-table (make-char-table nil))
(defvar ligature-font-with-big-arrow "Fira Code 11"
  "A valid font name for `set-frame-font'. Since this example uses eros.el
A font with a => ligature is necessary to trigger the bug.")
(require 'composite)

(let ((alist
       '(
         (?= . "=>") ; Matches =>
         )))
  (dolist (char-regexp alist)
    (set-char-table-range other-composition-ligature-table (car char-regexp)
                          `([,(cdr char-regexp) 0 font-shape-gstring]))))

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

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


;;;; Setting the font

;; =>
;; At this line
(set-frame-font ligature-font-with-big-arrow nil t)
;; Now you should see ligatures for =>
;; =>

;;;; Enabling the bug

;; C-x C-e at the end of this sexp will trigger infinite composition
(+ 2 3)

(provide 'ligature-bug)
;;; ligature-bug.el ends here

Re: Question about composite.c

[Eli Zaretskii]

> From: Gerry Agbobada <gagbobada@gmail.com>
> Date: Wed, 15 Apr 2020 00:43:59 +0200
> Cc: emacs-devel@gnu.org
> 
> It took me a good while, but I found a case where :
> - I can reproduce with =emacs -q=  (Reproducing infinite looping in
> composition_get_gstring with an empty non-nil string)
> - prettify-symbols-mode is not enabled
> 
> The example includes the code of eros.el package, which creates an
> overlay when calling eval-last-sexp. The overlay is supposed to start
> with `=>`, and when I have this in my composition-function-table it
> triggers the loop.
> As explained in the file linked below, with the cursor on `(+ 3 2)|`
> I'd expect `C-x C-e` to create an overlay with `=> 5`. Instead, it
> loops indefinitely and the only way out is to kill -9 emacs (kill
> -USR2 doesn't help)

Is the overlay a necessary part of the problem?  Also, is it necessary
to define a separate char-table, instead of using
composition-function-table?

The example looks quite complex, and I wonder whether you could
prepare a simplified variant that only included the parts that are
necessary?  That would make debugging this issue much easier.

> (let ((alist
>        '(
>          (?= . "=>") ; Matches =>
>          )))
>   (dolist (char-regexp alist)
>     (set-char-table-range other-composition-ligature-table (car char-regexp)
>                           `([,(cdr char-regexp) 0 font-shape-gstring]))))
> 
> (set-char-table-parent other-composition-ligature-table composition-function-table)
> 
> (setq-local composition-function-table other-composition-ligature-table)

From this, I don't think I understand why the problem is triggered by
a '+' character.  If I do the following in "emacs -Q":

  (set-char-table-range composition-function-table
			#x3d
			(list (vector "=>" 0 'compose-gstring-for-graphic)))

and then type (+ 2 3)C-x C-e, I don't get any loop, and no characters
related to the => ligatures anywhere.  What am I missing?

In what version of Emacs do you see these infloops, btw?

Re: Question about composite.c

[Gerry Agbobada]

> Is the overlay a necessary part of the problem?
Yes, because it's the overlay that tries to print a composed "=>" combination.


> Also, is it necessaryto define a separate char-table, instead of using
> composition-function-table?
> The example looks quite complex, and I wonder whether you could
> prepare a simplified variant that only included the parts that are
> necessary?  That would make debugging this issue much easier.
I agree that it might still be a little complex. I'll try to work on
this again to provide
the minimum amount of basic elisp functions code, but it's a little hard for me
to pindown the exact issue so I'm just making baby steps for the moment.

> From this, I don't think I understand why the problem is triggered by
> a '+' character.  If I do the following in "emacs -Q":
The problem is not triggered by the '+' character but by eros-mode
which will try to
output an overlay on the cursor with the content "=> 5".

> In what version of Emacs do you see these infloops, btw?
I was at e1f0e0892 (tip of emacs-27 on Wed Mar 25 21:54:21 2020 +0200 )
I will build from master right now to continue the investigation.

Thanks for your time answering me

Gerry AGBOBADA

Le sam. 25 avr. 2020 à 14:44, Eli Zaretskii <eliz@gnu.org> a écrit :
>
> > From: Gerry Agbobada <gagbobada@gmail.com>
> > Date: Wed, 15 Apr 2020 00:43:59 +0200
> > Cc: emacs-devel@gnu.org
> >
> > It took me a good while, but I found a case where :
> > - I can reproduce with =emacs -q=  (Reproducing infinite looping in
> > composition_get_gstring with an empty non-nil string)
> > - prettify-symbols-mode is not enabled
> >
> > The example includes the code of eros.el package, which creates an
> > overlay when calling eval-last-sexp. The overlay is supposed to start
> > with `=>`, and when I have this in my composition-function-table it
> > triggers the loop.
> > As explained in the file linked below, with the cursor on `(+ 3 2)|`
> > I'd expect `C-x C-e` to create an overlay with `=> 5`. Instead, it
> > loops indefinitely and the only way out is to kill -9 emacs (kill
> > -USR2 doesn't help)
>
> Is the overlay a necessary part of the problem?  Also, is it necessary
> to define a separate char-table, instead of using
> composition-function-table?
>
> The example looks quite complex, and I wonder whether you could
> prepare a simplified variant that only included the parts that are
> necessary?  That would make debugging this issue much easier.
>
> > (let ((alist
> >        '(
> >          (?= . "=>") ; Matches =>
> >          )))
> >   (dolist (char-regexp alist)
> >     (set-char-table-range other-composition-ligature-table (car char-regexp)
> >                           `([,(cdr char-regexp) 0 font-shape-gstring]))))
> >
> > (set-char-table-parent other-composition-ligature-table composition-function-table)
> >
> > (setq-local composition-function-table other-composition-ligature-table)
>
> From this, I don't think I understand why the problem is triggered by
> a '+' character.  If I do the following in "emacs -Q":
>
>   (set-char-table-range composition-function-table
>                         #x3d
>                         (list (vector "=>" 0 'compose-gstring-for-graphic)))
>
> and then type (+ 2 3)C-x C-e, I don't get any loop, and no characters
> related to the => ligatures anywhere.  What am I missing?
>
> In what version of Emacs do you see these infloops, btw?

Re: Question about composite.c

[Gerry Agbobada]

I cannot reproduce the bug even with my complex example on master, but
still can reproduce it on 05089a4d (tip of emacs-27 last week).

Bissecting will take a while but I'll try to find what commit changed
the behaviour so I can understand a bit more.

Gerry AGBOBADA

Le sam. 25 avr. 2020 à 15:06, Gerry Agbobada <gagbobada@gmail.com> a écrit :
>
> > Is the overlay a necessary part of the problem?
> Yes, because it's the overlay that tries to print a composed "=>" combination.
>
>
> > Also, is it necessaryto define a separate char-table, instead of using
> > composition-function-table?
> > The example looks quite complex, and I wonder whether you could
> > prepare a simplified variant that only included the parts that are
> > necessary?  That would make debugging this issue much easier.
> I agree that it might still be a little complex. I'll try to work on
> this again to provide
> the minimum amount of basic elisp functions code, but it's a little hard for me
> to pindown the exact issue so I'm just making baby steps for the moment.
>
> > From this, I don't think I understand why the problem is triggered by
> > a '+' character.  If I do the following in "emacs -Q":
> The problem is not triggered by the '+' character but by eros-mode
> which will try to
> output an overlay on the cursor with the content "=> 5".
>
> > In what version of Emacs do you see these infloops, btw?
> I was at e1f0e0892 (tip of emacs-27 on Wed Mar 25 21:54:21 2020 +0200 )
> I will build from master right now to continue the investigation.
>
> Thanks for your time answering me
>
> Gerry AGBOBADA
>
> Le sam. 25 avr. 2020 à 14:44, Eli Zaretskii <eliz@gnu.org> a écrit :
> >
> > > From: Gerry Agbobada <gagbobada@gmail.com>
> > > Date: Wed, 15 Apr 2020 00:43:59 +0200
> > > Cc: emacs-devel@gnu.org
> > >
> > > It took me a good while, but I found a case where :
> > > - I can reproduce with =emacs -q=  (Reproducing infinite looping in
> > > composition_get_gstring with an empty non-nil string)
> > > - prettify-symbols-mode is not enabled
> > >
> > > The example includes the code of eros.el package, which creates an
> > > overlay when calling eval-last-sexp. The overlay is supposed to start
> > > with `=>`, and when I have this in my composition-function-table it
> > > triggers the loop.
> > > As explained in the file linked below, with the cursor on `(+ 3 2)|`
> > > I'd expect `C-x C-e` to create an overlay with `=> 5`. Instead, it
> > > loops indefinitely and the only way out is to kill -9 emacs (kill
> > > -USR2 doesn't help)
> >
> > Is the overlay a necessary part of the problem?  Also, is it necessary
> > to define a separate char-table, instead of using
> > composition-function-table?
> >
> > The example looks quite complex, and I wonder whether you could
> > prepare a simplified variant that only included the parts that are
> > necessary?  That would make debugging this issue much easier.
> >
> > > (let ((alist
> > >        '(
> > >          (?= . "=>") ; Matches =>
> > >          )))
> > >   (dolist (char-regexp alist)
> > >     (set-char-table-range other-composition-ligature-table (car char-regexp)
> > >                           `([,(cdr char-regexp) 0 font-shape-gstring]))))
> > >
> > > (set-char-table-parent other-composition-ligature-table composition-function-table)
> > >
> > > (setq-local composition-function-table other-composition-ligature-table)
> >
> > From this, I don't think I understand why the problem is triggered by
> > a '+' character.  If I do the following in "emacs -Q":
> >
> >   (set-char-table-range composition-function-table
> >                         #x3d
> >                         (list (vector "=>" 0 'compose-gstring-for-graphic)))
> >
> > and then type (+ 2 3)C-x C-e, I don't get any loop, and no characters
> > related to the => ligatures anywhere.  What am I missing?
> >
> > In what version of Emacs do you see these infloops, btw?

Re: Question about composite.c

[Gerry Agbobada]

I can confirm the bug is fixed by
commit fe903c5ab7354b97f80ecf1b01ca3ff1027be446
Author: Eli Zaretskii
Date: Sat Feb 8 15:41:36 2020 +0200
Allow composition of pure-ASCII strings in the mode line

* src/composite.c (Fcomposition_get_gstring): Allow unibyte strings if
they are pure
ASCII, by copying text into a multibyte string.


If it's possible to backport this commit in emacs 27 I think it would
save a lot of headaches.
I still don't know what exactly I did wrong with that combination of
overlays and ligatures,
but since it's fixed in master I won't look a lot more into that ; I
will just try to understand what
this patch does.

Best regards,

Gerry AGBOBADA

Le sam. 25 avr. 2020 à 15:29, Gerry Agbobada <gagbobada@gmail.com> a écrit :
>
> I cannot reproduce the bug even with my complex example on master, but
> still can reproduce it on 05089a4d (tip of emacs-27 last week).
>
> Bissecting will take a while but I'll try to find what commit changed
> the behaviour so I can understand a bit more.
>
> Gerry AGBOBADA
>
> Le sam. 25 avr. 2020 à 15:06, Gerry Agbobada <gagbobada@gmail.com> a écrit :
> >
> > > Is the overlay a necessary part of the problem?
> > Yes, because it's the overlay that tries to print a composed "=>" combination.
> >
> >
> > > Also, is it necessaryto define a separate char-table, instead of using
> > > composition-function-table?
> > > The example looks quite complex, and I wonder whether you could
> > > prepare a simplified variant that only included the parts that are
> > > necessary?  That would make debugging this issue much easier.
> > I agree that it might still be a little complex. I'll try to work on
> > this again to provide
> > the minimum amount of basic elisp functions code, but it's a little hard for me
> > to pindown the exact issue so I'm just making baby steps for the moment.
> >
> > > From this, I don't think I understand why the problem is triggered by
> > > a '+' character.  If I do the following in "emacs -Q":
> > The problem is not triggered by the '+' character but by eros-mode
> > which will try to
> > output an overlay on the cursor with the content "=> 5".
> >
> > > In what version of Emacs do you see these infloops, btw?
> > I was at e1f0e0892 (tip of emacs-27 on Wed Mar 25 21:54:21 2020 +0200 )
> > I will build from master right now to continue the investigation.
> >
> > Thanks for your time answering me
> >
> > Gerry AGBOBADA
> >
> > Le sam. 25 avr. 2020 à 14:44, Eli Zaretskii <eliz@gnu.org> a écrit :
> > >
> > > > From: Gerry Agbobada <gagbobada@gmail.com>
> > > > Date: Wed, 15 Apr 2020 00:43:59 +0200
> > > > Cc: emacs-devel@gnu.org
> > > >
> > > > It took me a good while, but I found a case where :
> > > > - I can reproduce with =emacs -q=  (Reproducing infinite looping in
> > > > composition_get_gstring with an empty non-nil string)
> > > > - prettify-symbols-mode is not enabled
> > > >
> > > > The example includes the code of eros.el package, which creates an
> > > > overlay when calling eval-last-sexp. The overlay is supposed to start
> > > > with `=>`, and when I have this in my composition-function-table it
> > > > triggers the loop.
> > > > As explained in the file linked below, with the cursor on `(+ 3 2)|`
> > > > I'd expect `C-x C-e` to create an overlay with `=> 5`. Instead, it
> > > > loops indefinitely and the only way out is to kill -9 emacs (kill
> > > > -USR2 doesn't help)
> > >
> > > Is the overlay a necessary part of the problem?  Also, is it necessary
> > > to define a separate char-table, instead of using
> > > composition-function-table?
> > >
> > > The example looks quite complex, and I wonder whether you could
> > > prepare a simplified variant that only included the parts that are
> > > necessary?  That would make debugging this issue much easier.
> > >
> > > > (let ((alist
> > > >        '(
> > > >          (?= . "=>") ; Matches =>
> > > >          )))
> > > >   (dolist (char-regexp alist)
> > > >     (set-char-table-range other-composition-ligature-table (car char-regexp)
> > > >                           `([,(cdr char-regexp) 0 font-shape-gstring]))))
> > > >
> > > > (set-char-table-parent other-composition-ligature-table composition-function-table)
> > > >
> > > > (setq-local composition-function-table other-composition-ligature-table)
> > >
> > > From this, I don't think I understand why the problem is triggered by
> > > a '+' character.  If I do the following in "emacs -Q":
> > >
> > >   (set-char-table-range composition-function-table
> > >                         #x3d
> > >                         (list (vector "=>" 0 'compose-gstring-for-graphic)))
> > >
> > > and then type (+ 2 3)C-x C-e, I don't get any loop, and no characters
> > > related to the => ligatures anywhere.  What am I missing?
> > >
> > > In what version of Emacs do you see these infloops, btw?

Re: Question about composite.c

[Eli Zaretskii]

> From: Gerry Agbobada <gagbobada@gmail.com>
> Date: Sat, 25 Apr 2020 15:53:49 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> I can confirm the bug is fixed by
> commit fe903c5ab7354b97f80ecf1b01ca3ff1027be446
> Author: Eli Zaretskii
> Date: Sat Feb 8 15:41:36 2020 +0200
> Allow composition of pure-ASCII strings in the mode line
> 
> * src/composite.c (Fcomposition_get_gstring): Allow unibyte strings if
> they are pure
> ASCII, by copying text into a multibyte string.

Thanks, that was my guess.

> If it's possible to backport this commit in emacs 27 I think it would
> save a lot of headaches.

Sorry, the change is somewhat risky, and that's why it went to master.

Re: Question about composite.c

[Gerry Agbobada]

No problem, I'll keep looking at the code and I'll try to report the issue
properly (with a bug and on the correct mailing list) if I ever manage to
either reproduce it on master or find the exact code path I follow in
my example.


Gerry AGBOBADA

Le sam. 25 avr. 2020 à 17:23, Eli Zaretskii <eliz@gnu.org> a écrit :
>
> > From: Gerry Agbobada <gagbobada@gmail.com>
> > Date: Sat, 25 Apr 2020 15:53:49 +0200
> > Cc: Emacs developers <emacs-devel@gnu.org>
> >
> > I can confirm the bug is fixed by
> > commit fe903c5ab7354b97f80ecf1b01ca3ff1027be446
> > Author: Eli Zaretskii
> > Date: Sat Feb 8 15:41:36 2020 +0200
> > Allow composition of pure-ASCII strings in the mode line
> >
> > * src/composite.c (Fcomposition_get_gstring): Allow unibyte strings if
> > they are pure
> > ASCII, by copying text into a multibyte string.
>
> Thanks, that was my guess.
>
> > If it's possible to backport this commit in emacs 27 I think it would
> > save a lot of headaches.
>
> Sorry, the change is somewhat risky, and that's why it went to master.