[PATCH] hack-one-local-variable use lexical-binding

[PATCH] hack-one-local-variable use lexical-binding

[Tom Gillespie]

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

Hi,
   A small patch with a long preamble and a few questions about the
   best approach. Best!
Tom

=eval:= local variables currently cannot use lexical-binding

An example.

#+begin_src org
# -*- lexical-binding: t -*-
,#+name: code-block
,#+begin_src elisp :results none :lexical yes
(setq testv "no")
,#+end_src

Local Variables:
eval: (let ((testv "yes")) (org-sbe code-block) (message "%s" testv))
End:
#+end_src

There are two possible solutions. One is to pass t as LEXICAL at all
times. The other is to pass lexical-binding so that the behavior can
be controlled via =-*- lexical-binding: t -*-= on the prop line.

I'm not sure what the best approach is here. According to the manual
https://www.gnu.org/software/emacs/manual/html_node/elisp/Using-Lexical-Binding.html
other special evaluation contexts such as --eval, M-:, *scratch*, and
*ielm* have all already switched over to lexical binding by default.
Should local variables list eval variables follow the behavior of
these other special execution contexts? Or, since they are part of a
file should they follow the elisp file convention which is currently
to follow the lexical-binding local variable?

Despite personally having a use case for dynamic binding in this
context, from a sanity point of view I think setting LEXICAL to t at
all times may be the correct thing to do since with dynamic binding it
is possible for a function to overwrite a let bound variable in such a
way that the original value can never be recovered in the calling
scope.  However, that is sort of the point of dynamic binding, and it
would be a major breaking change to the default behavior, and it would
still not be configurable. Given that I also have no idea what the
actual impact of that change would be on users, using the local value
of lexical-binding seems to follow the principle of least surprise and
seems to be future proof if the default value of lexical-binding is
ever changed to t in the future (unlikely?).

One impact of this change is that =-*- lexical-binding: t -*-= on the
prop line will have meaning outside of Emacs lisp files and will have
meaning in any file that makes use of an =eval:= local variable.

As far as I can tell, passing =lexical-binding= works as expected.
=lexical-binding= needs to be set on the prop-line as usual, but can
also be set via an =eval:= local variable before any expression that
needs lexical binding in the local variables list. This is probably ok
because the use of =lexical-binding= in this context is not about the
evaluation of the current file, but rather about the evaluation of
further =eval:= local variables. Using =setq-local= to set
=lexical-binding= to =nil= in the strange case where someone wants
=lexical-binding= for their elisp file, but not when dealing with the
=eval:= local variables is also possible.

[-- Attachment #2: 0001-hack-one-local-variable-use-lexical-binding.patch --]
[-- Type: text/x-patch, Size: 829 bytes --]

From 5201892779738787a7dbe2d22735d2efb616597f Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Wed, 25 Nov 2020 21:35:04 -0500
Subject: [PATCH] hack-one-local-variable use lexical-binding

* lisp/files.el (hack-one-local-variable): pass eval lexical-binding
---
 lisp/files.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 49c9e5d18d..c1c18ec58d 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3984,7 +3984,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 lexical-binding)))
     (_
      (hack-one-local-variable--obsolete var)
      ;; Make sure the string has no text properties.
-- 
2.26.2

Re: [PATCH] hack-one-local-variable use lexical-binding

[Stefan Monnier]

> There are two possible solutions. One is to pass t as LEXICAL at all
> times. The other is to pass lexical-binding so that the behavior can
> be controlled via =-*- lexical-binding: t -*-= on the prop line.

I think we should just always pass `t` to `eval` there, yes.
The other option is over-engineered.


        Stefan

Re: [PATCH] hack-one-local-variable use lexical-binding

[Tom Gillespie]

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

> I think we should just always pass `t` to `eval` there, yes.
> The other option is over-engineered.

Agreed. Here is the updated patch. I'm guessing this will need a NEWS item?

Best,
Tom

[-- Attachment #2: 0001-hack-one-local-variable-eval-with-LEXICAL-t.patch --]
[-- Type: text/x-patch, Size: 884 bytes --]

From d1b261faaed22f92a2c61ce43236a77221b6ce74 Mon Sep 17 00:00:00 2001
From: Tom Gillespie <tgbugs@gmail.com>
Date: Wed, 25 Nov 2020 23:37:36 -0500
Subject: [PATCH] hack-one-local-variable eval with LEXICAL t

* lisp/files.el (hack-one-local-variable): call eval with LEXICAL t

Change the behavior of eval: local variables to use lexical binding.
---
 lisp/files.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/files.el b/lisp/files.el
index 49c9e5d18d..f6c1c44718 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3984,7 +3984,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.
-- 
2.26.2

Re: [PATCH] hack-one-local-variable use lexical-binding

[Eli Zaretskii]

On November 26, 2020 6:47:44 AM GMT+02:00, Tom Gillespie <tgbugs@gmail.com> wrote:
> > I think we should just always pass `t` to `eval` there, yes.
> > The other option is over-engineered.
> 
> Agreed. Here is the updated patch. I'm guessing this will need a NEWS
> item?

This would be a backward-incompatible change, wouldn't it?  I'd prefer a backward compatible solution.  For example, how about a separate lexical-eval clause, which will do this  leaving eval to work as it did before?

And yeas  a NEWS item is necessary regardless.

Thanks.

Re: [PATCH] hack-one-local-variable use lexical-binding

[Stefan Monnier]

> This would be a backward-incompatible change, wouldn't it?

I've made exactly this kind of incompatible change many times over
between Emacs-24 and now.  I don't think we should worry about it.

> I'd prefer a backward compatible solution.  For example, how about
> a separate lexical-eval clause, which will do this leaving eval to
> work as it did before?

As I said, that's over-engineering.


        Stefan

Re: [PATCH] hack-one-local-variable use lexical-binding

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org, Tom Gillespie <tgbugs@gmail.com>
> Date: Thu, 26 Nov 2020 09:21:22 -0500
> 
> > This would be a backward-incompatible change, wouldn't it?
> 
> I've made exactly this kind of incompatible change many times over
> between Emacs-24 and now.  I don't think we should worry about it.
> 
> > I'd prefer a backward compatible solution.  For example, how about
> > a separate lexical-eval clause, which will do this leaving eval to
> > work as it did before?
> 
> As I said, that's over-engineering.

We disagree.

Re: [PATCH] hack-one-local-variable use lexical-binding

[Tom Gillespie]

> > > This would be a backward-incompatible change, wouldn't it?
> >
> > I've made exactly this kind of incompatible change many times over
> > between Emacs-24 and now.  I don't think we should worry about it.
> >
> > > I'd prefer a backward compatible solution.  For example, how about
> > > a separate lexical-eval clause, which will do this leaving eval to
> > > work as it did before?
> >
> > As I said, that's over-engineering.
>
> We disagree.

After reflecting on this I think that there is an important difference
between the local variables list and the other special evaluation
contexts which might make this not over-engineered. In the M-:, ielm,
and scratch cases, there isn't an existing file, this is particularly
relevant where it would be important to always have the same behavior
in the minibuffer regardless of file. However, there is a file in the
case of an eval local variable. For consistency this means that the
file should have control over whether lexical or dynamic bindings is
used. As noted in my original email, the current implementation would
also allow users to control the lexical binding of the local variables
independent of the rest of the file. While I personally have never
encountered a case where someone explicitly sets lexical-binding: nil,
I imagine that it would be surprising if elisp code in a file with
that set was irreversibly forced into lexical mode despite the fact
that the configuration option already must be present in the file.
Obviously this would not be the case for non-elisp files, but since
the prop line lexical-binding infrastructure is going to be around for
the foreseeable future I'm not sure what there is to lose by simply
passing lexical-binding instead of t. Best,
Tom

Re: [PATCH] hack-one-local-variable use lexical-binding

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: emacs-devel@gnu.org, Tom Gillespie <tgbugs@gmail.com>
>> Date: Thu, 26 Nov 2020 09:21:22 -0500
>>
>> > This would be a backward-incompatible change, wouldn't it?
>>
>> I've made exactly this kind of incompatible change many times over
>> between Emacs-24 and now.  I don't think we should worry about it.
>>
>> > I'd prefer a backward compatible solution.  For example, how about
>> > a separate lexical-eval clause, which will do this leaving eval to
>> > work as it did before?
>>
>> As I said, that's over-engineering.
>
> We disagree.

Once we use lexical-binding t by default in all other cases, it would be
surprising for it to default to nil in this case.  For me, the most
important thing is therefore to have a clear idea how we can make file
local `eval' use lexical-binding by default at some point in the future.

If Eli insists that we can't make this backwards incompatible change
now, I would suggest instead to introduce `non-lexical-eval' or
`dynamic-eval' and document that any use of plain `eval' that relies on
lexical-binding nil is now deprecated.

In a future version, we can then make the proposed backwards
incompatible change, once users have had sufficient time to switch to
`dynamic-eval'.  This change could be made, for example, when we switch
to use lexical-binding by default in .el files.

(We could also provide `lexical-eval', if we want to.)

Re: [PATCH] hack-one-local-variable use lexical-binding

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Sun, 29 Nov 2020 03:43:04 -0600
> Cc: tgbugs@gmail.com, emacs-devel@gnu.org
> 
> Once we use lexical-binding t by default in all other cases, it would be
> surprising for it to default to nil in this case.

Once lexical-binding becomes t by default, 'eval' will use
lexical-binding.

> For me, the most important thing is therefore to have a clear idea
> how we can make file local `eval' use lexical-binding by default at
> some point in the future.

My idea is to make it do that by default when lexical-binding is t by
default; until then, if someone wants that in file-local variables, he
or she can use a special construct.

IOW, this change will be a backward-incompatible one no matter what we
do.  IMO, it makes much more sense to make such a change when we
switch to lexical-binding by default rather than now.

> If Eli insists that we can't make this backwards incompatible change
> now, I would suggest instead to introduce `non-lexical-eval' or
> `dynamic-eval' and document that any use of plain `eval' that relies on
> lexical-binding nil is now deprecated.

I don't mind introducing dynamic-eval if people think it could be of
use.  I don't think it's a must, but I won't object to it.

Re: [PATCH] hack-one-local-variable use lexical-binding

[Tom Gillespie]

> Once lexical-binding becomes t by default, 'eval' will use
> lexical-binding.

So ultimately better not to rely on the default value of
lexical-binding at all. Is there a way to pass LEXICAL t unless a file
local variable explicitly sets lexical-binding to nil? This would be
equivalent to fast forwarding this particular part of the
implementation in time as if it were already in the future where
lexical-binding was true by default. Almost certainly not worth the
complexity.

> IOW, this change will be a backward-incompatible one no matter what we
> do.  IMO, it makes much more sense to make such a change when we
> switch to lexical-binding by default rather than now.

This makes sense. I don't think there is a particular hurry on this,
but if there were a simple way to make this subsystem behave as if
lexical-binding were t by default (sounds like the answer there will
be no) then the change could be made now and the breaking of the
default behavior would still happen only once. If the additional
changes needed to do that are too extensive, then it is probably
better just to wait.

The only outstanding question to me is whether users would be able to
control whether the eval local variable uses lexical binding at all
even in the future. I can imagine that when the switch to
lexical-binding being t by default happens there will be a need for an
escape hatch

> can use a special construct.

It crosses my mind that one potential solution would be lexical-eval:
and dynamic-eval: local variables, but this seems like it might not be
worth the added complexity. On the other hand, it would make it
completely unambiguous to a user inspecting a file what behavior will
be and make that behavior independent of any differences in local
settings or future changes to defaults. In the few cases where an
author knows lexical vs dynamic matters, it would definitely be
simpler for the author of the local variables to say "this always
needs to be dynamically evaluated" and then not have to worry about it
in the future.

Best,
Tom

Re: [PATCH] hack-one-local-variable use lexical-binding

[Stefan Monnier]

> The only outstanding question to me is whether users would be able to
> control whether the eval local variable uses lexical binding at all
> even in the future. I can imagine that when the switch to
> lexical-binding being t by default happens there will be a need for an
> escape hatch

The lexical-binding dialect of ELisp already offers both dynamically
scoped bindings and statically scoped bindings (controlled by the use
of things like `defvar` and `dlet` to declare that a var should use dynamic
scoping).


        Stefan