bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

[Michael Heerdegen]

Hello,

this is the second time I have stumbled across this and it is annoying -
so here is the story:

The template of the code I was using is like this:

#+begin_src emacs-lisp
;; -*- lexical-binding: t -*-

(defun test ()
  (interactive)
  (funcall (or my-do-with-test-process-fun #'identity)
           (start-process "test-process" "foo" "xterm")))

(defvar my-do-with-test-process-fun nil)
#+end_src

I want to use `my-do-with-test-process-fun' as a file local variable.  I
try like this:

testfile.txt:
| File contents ...
| ...
|
| Local Variables:
| my-do-with-test-process-fun: (lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test"))))
| End:

(I hope mailing stuff doesn't break the overlong line defining
`my-do-with-test-process-fun'.)

When I open that file and try M-x test I get this error:

| Debugger entered--Lisp error: (void-variable #:v)
|   (process-sentinel #:v)
|   (lambda nil (process-sentinel #:v))()
|   advice--add-function(:before ((lambda nil (process-sentinel #:v)) lambda (gv--val) (set-process-sentinel #:v gv--val)) (lambda (&rest _) (message "Test")) nil)
|   (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test")))
|   (lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test"))))(#<process test-process>)
|   funcall((lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test")))) #<process test-process>)
|   test()


(1) I suspect that this happens because the lack of lexical binding
"somewhere".  Enabling lexical binding mode in `testfile.txt' alone
doesn't help.  OTOH, setting the file local variable like this:

| Local Variables:
| eval: (setq-local my-do-with-test-process-fun (eval (lambda (p) (add-function ...) t)))
| End:

makes `M-x test` work.  Is this all expected so far?


(2) If lack of lexical binding is the culprit: I see that `add-function'
uses `gv-ref' and that warns about uses in dynamically binding Elisp.
Should `add-function' warn about such a restriction as well?


(3) And I wonder: now that lexical binding Elisp gets more common,
should file local variables be set using a lexically binding environment
-- or at least when the file itself specifies lexical binding mode?


TIA,

Michael.



In GNU Emacs 28.0.50 (build 15, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-06-21 built on drachen
Repository revision: 50a59b9af1517f24fca60feab37140f2b35ea5ac
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: Debian GNU/Linux bullseye/sid

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LCMS2 LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP
SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_ALL: de_DE.utf8
  value of $LC_COLLATE: C
  value of $LC_TIME: C
  value of $LANG: de_DE.utf8
  locale-coding-system: utf-8-unix

bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> | my-do-with-test-process-fun: (lambda (p) (add-function :before
> | (process-sentinel p) (lambda (&rest _) (message "Test"))))

Remember, file-local vars set a var to a value, there's no evaluation
going on, so this sets `my-do-with-test-process-fun` to a list whose car
is the symbol `lambda`.

> (2) If lack of lexical binding is the culprit: I see that `add-function'
> uses `gv-ref' and that warns about uses in dynamically binding Elisp.
> Should `add-function' warn about such a restriction as well?

It might (when you do the `M-x test`); have you checked *Messages*?

> (3) And I wonder: now that lexical binding Elisp gets more common,
> should file local variables be set using a lexically binding environment
> -- or at least when the file itself specifies lexical binding mode?

I think the patch below would be in order, yes, but it won't help
your example unless you change it to something like:

    eval: (setq-local my-do-with-test-process-fun: (lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test")))))


        Stefan


diff --git a/lisp/files.el b/lisp/files.el
index 5d2fe0a77b..04db0faffd 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -4041,7 +4041,7 @@ hack-one-local-variable
     ('eval
      (pcase val
        (`(add-hook ',hook . ,_) (hack-one-local-variable--obsolete hook)))
-     (save-excursion (eval val)))
+     (save-excursion (eval val t)))
     (_
      (hack-one-local-variable--obsolete var)
      ;; Make sure the string has no text properties.

bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Remember, file-local vars set a var to a value, there's no evaluation
> going on, so this sets `my-do-with-test-process-fun` to a list whose
> car is the symbol `lambda`.

Grml...

> It might (when you do the `M-x test`); have you checked *Messages*?

No, but I guess you have ;-)  Ok, that's the maximum it can do.

> -     (save-excursion (eval val)))
> +     (save-excursion (eval val t)))

I would certainly welcome that.  How controversial would doing that be
(and would `(eval val lexical-binding)' be less controversial then)?


Thanks,

Michael.

bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> -     (save-excursion (eval val)))
>> +     (save-excursion (eval val t)))
> I would certainly welcome that.

Pushed.  I understand it doesn't directly satisfies your original
request, but that seems harder to do.

This said, technically you *can* get exactly what you asked for with
something like:

    Local Variables:
    my-do-with-test-process-fun: #[257 "\300\301\^B\302\303\^B\"\302\304\^C\"B\262\^A\305\306$\207" [advice--add-function :before make-closure #[0 "\301\300!\207" [V0 process-sentinel] 2] #[257 "\301\300\^B\"\207" [V0 set-process-sentinel] 4 "\n\n(fn GV--VAL)"] #[128 "\300\301!\207" [message "Test"] 3 "\n\n(fn &rest _)"] nil] 8 "\n\n(fn P)"]
    End:

which you can generate with:

    M-: (let ((lexical-binding t) (print-escape-newlines t)) (insert (format "%S" (byte-compile '(lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test"))))))))

I wouldn't encourage you to do that, tho.


        Stefan

bug#49163: 28.0.50; Dynbind add-function and lambdas as file vars

[Michael Heerdegen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> >> -     (save-excursion (eval val)))
> >> +     (save-excursion (eval val t)))
>
> Pushed.  I understand it doesn't directly satisfies your original
> request, but that seems harder to do.

Thanks, Stefan.

I can live with what we have now.

> This said, technically you *can* get exactly what you asked for with
> something like:
>
>     Local Variables:
>     my-do-with-test-process-fun: #[257 "\300\301\^B\302\303\^B\"\302\304\^C\"B\262\^A\305\306$\207" [advice--add-function :before make-closure #[0 "\301\300!\207" [V0 process-sentinel] 2] #[257 "\301\300\^B\"\207" [V0 set-process-sentinel] 4 "\n\n(fn GV--VAL)"] #[128 "\300\301!\207" [message "Test"] 3 "\n\n(fn &rest _)"] nil] 8 "\n\n(fn P)"]
>     End:
>
> which you can generate with:
>
>     M-: (let ((lexical-binding t) (print-escape-newlines t)) (insert (format "%S" (byte-compile '(lambda (p) (add-function :before (process-sentinel p) (lambda (&rest _) (message "Test"))))))))

Good to know that this works.  The main problem is that it's not human
readable and doesn't contain the "source", so it's not less complex
than using `eval' at the end.


Michael.