bug#67390: 28; shorthands-font-lock-shorthands assumes shorthand uses same separator

[Joseph Turner via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

João Távora <joaotavora@gmail.com> writes:

> Sorry, this flew under the radar.  I thought I had already pushed to master
> but didn't.  So I went through the commits again, addressed your concerns, and
> applied your suggestions.  Pushed to master now.

Thank you!

> On Sat, Feb 3, 2024 at 7:10 AM Joseph Turner <joseph@ushin.org> wrote:
>
>
>>> -[ \t]*%s\\([ \t;]+\\|$\\)")
>>> -                           (regexp-opt (mapcar 'cadr fnlist) t)))
>>> +[ \t]*\\(\\(?:\\sw\\|\\s_\\)+\\)\\([ \t;]+\\|$\\)"))
>>
>> Would you explain what this regexp is intended to match?
>
> A very complicated one, right?  Well ask the author, but I think it's intended
> to find many definition-like forms.  No idea why this is  done with regexps and
> not with 'read' as it is a classical parsing pitfall in the long run.
> Maybe there
> was a reason.
>
> Anyway, I just added a bit of logic so that it considers
> read-symbol-shorthands if
> there are any.

That makes sense.

>> Are you able to reproduce this on your machine?
>
> Yes, and I fixed it.
>
>> ...then this form will allocate two strings almost as long as the file.
>>
>> Here's an alternative hack attempting to uncomment and read the minimum:
>
> Thanks, I think that's a good idea and I added a commit in your name.

Thanks!

>> A couple typo nits on the commit message of "Improve
>> shorthands-font-lock-shorthands (bug#67390)":
>>
>> -  h//thingy     ; hilits "//"       reads to 'hyperdrive--thingy'
>> +  h//thingy     ; hilits "h//"       reads to 'hyperdrive--thingy'
>>
>> -  Co-authored-by: João Távora <joaotavora@gmail.com>
>> +  Co-authored-by: Joseph Turner <joseph@breatheoutbreathe.in>
>
> I fixed these, too.  If you succesfully test this, I think we can close this bug
> (and the other ones, too).

I'm still reproducing the check-declare bug on my machine. It appears
that binding `enable-local-variables' to nil around the call to
`hack-local-variables' means that `read-symbol-shorthands' is not set.
Can we bind `enable-local-variables' to `:safe' instead?

(let (
      ;; (enable-local-variables t)   ; works
      ;; (enable-local-variables)     ; doesn't work
      (enable-local-variables :safe)  ; works
      )
  (with-temp-buffer
    (insert-file-contents "~/.local/src/hyperdrive.el/hyperdrive-lib.el")
    (hack-local-variables)
    read-symbol-shorthands))

See attached patch.

There's no way to hack just a single file- or dir-local variable, right?

Thank you!

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Bind-enable-local-variables-to-safe-for-shorthands-b.patch --]
[-- Type: text/x-diff, Size: 1667 bytes --]

From f301d839031f78b303d698d7bca1f0b27264d2d8 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Sat, 3 Feb 2024 11:56:31 -0800
Subject: [PATCH] ;  Bind enable-local-variables to :safe for shorthands
 (bug#67523)

* lisp/emacs-lisp/check-declare.el (check-declare-scan):
(check-declare-verify):
---
 lisp/emacs-lisp/check-declare.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/emacs-lisp/check-declare.el b/lisp/emacs-lisp/check-declare.el
index a6d1a330d90..eff37828f65 100644
--- a/lisp/emacs-lisp/check-declare.el
+++ b/lisp/emacs-lisp/check-declare.el
@@ -87,7 +87,7 @@ don't know how to recognize (e.g. some macros)."
       (insert-file-contents file)
       ;; Ensure shorthands available, as we will be `read'ing Elisp
       ;; (bug#67523)
-      (let (enable-local-variables) (hack-local-variables))
+      (let ((enable-local-variables :safe)) (hack-local-variables))
       ;; FIXME we could theoretically be inside a string.
       (while (re-search-forward "^[ \t]*\\((declare-function\\)[ \t\n]" nil t)
         (let ((pos (match-beginning 1)))
@@ -152,7 +152,7 @@ is a string giving details of the error."
             ;; If in Elisp, ensure syntax and shorthands available
             ;; (bug#67523)
             (set-syntax-table emacs-lisp-mode-syntax-table)
-            (let (enable-local-variables) (hack-local-variables)))
+            (let ((enable-local-variables :safe)) (hack-local-variables)))
           ;; defsubst's don't _have_ to be known at compile time.
           (setq re (if cflag
                        (format "^[ \t]*\\(DEFUN\\)[ \t]*([ \t]*\"%s\""
-- 
2.41.0