bug#39190: 28.0.50; two buffers with same buffer-file-name (diff-syntax-fontify-props)

[Juri Linkov <juri@linkov.net>]

Now I tried to repeat this problem with eglot and got such backtrace:

Debugger entered--Lisp error: (wrong-type-argument arrayp nil)
  substring(nil 0 1)
  file-truename(nil)
  eglot--path-to-uri(nil)
  eglot--TextDocumentIdentifier()
  eglot--signal-textDocument/didClose()
  kill-buffer(#<buffer  *temp*-492063>)
  (and (buffer-name temp-buffer) (kill-buffer temp-buffer))
  (unwind-protect (progn (insert text) (diff-syntax-fontify-props ...
  (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (insert text) (diff-syntax-fontify-props ...
  (let ((temp-buffer (generate-new-buffer " *temp*"))) (save-current-buffer (set-buffer temp-buffer) (unwind-protect (progn (insert text) ...
  (cond ((and file diff-default-directory ...) ...) ...)
  (let ((file (car (diff-hunk-file-names old)))) ...)
  (or (if (and diff-vc-backend (not (eq diff-font-lock-syntax 'hunk-only))) ...) ...)
  (let* ((hunk (buffer-substring-no-properties beg end)) ...) ...
  diff-syntax-fontify-hunk(10530 14909 t)
  diff-syntax-fontify(10530 14909)
  #f(compiled-function (beg end) #<bytecode 0x1cb64d70b7d4e0e5>)(10530 14909)
  diff--iterate-hunks(12505 #f(compiled-function (beg end) #<bytecode 0x1cb64d70b7d4e0e5>))
  diff--font-lock-syntax(12505)
  font-lock-fontify-keywords-region(11965 12505 nil)
  font-lock-default-fontify-region(11980 12480 nil)
  font-lock-fontify-region(11980 12480)
  #f(compiled-function (fun) #<bytecode 0x303b13b2afd8864>)(font-lock-fontify-region)
  run-hook-wrapped(#f(compiled-function (fun) #<bytecode 0x303b13b2afd8864>) font-lock-fontify-region)
  jit-lock--run-functions(11980 12480)
  jit-lock-fontify-now(11980 12480)
  jit-lock-function(11980)
  redisplay_internal\ \(C\ function\)()

>> Stefan, do you think diff-syntax-fontify-props should let-bind
>> after-change-major-mode-hook to nil to not allow running this hook
>> on an internal buffer.
>
> No, that's definitely not the right fix.  It's at best a weak
> workaround, but it won't handle the case where the code that depends on
> `buffer-file-name` is placed on `foo-mode-hook` rather than on
> `after-change-major-mode-hook`.

In fact, eglot recommends adding such hooks:

  (add-hook 'foo-mode-hook 'eglot-ensure)

where eglot-ensure relies on buffer-file-name, so such fix is not possible indeed.

>>> If I read correctly, diff-syntax-fontify-props sets buffer-file-name of
>>> a temporary buffer to an existing one.  This is not necessarily a bug,
>>> but it definitely looks strange that we have two buffers with different
>>> contents and the same buffer-file-name.
>
> Yes, it's definitely borderline.  I remember feeling a bit uneasy about
> it at the time.  Maybe a better fix would be to be able to pass the
> file-name to `set-auto-mode` (or some new function created for that
> purpose) as an argument (instead of passing it via dynamic scoping in
> `buffer-file-name`).

The problem is that the right value of buffer-file-name is required
also by other more deep functions that set local variables like
dir-locals-collect-variables.

> BTW, I see somewhat similar code in `xref--collect-matches`, so maybe
> there's a need for a more general solution (haven't looked any further,
> but I'd be surprised if there aren't other cases).

Another example is mm-display-inline-fontify in gnus/mm-view.el,
so the problem is more widespread, and perhaps not much could be done
more than already using temporary buffers as an indication
for external packages that these buffers are for internal use only.

>> Or it's the responsibility of the eglot package to check for the
>> leading space in the buffer name when its after-change-major-mode-hook
>> is called?
>
> The eglot package could protect itself from such things by making its
> `after-change-major-mode-hook` just add the buffer to a global var, and
> then process this global var in `post-command-hook` or in a timer.

Felician, do you think that the eglot package should take more care
to protect itself, so this report could be closed?