bug#44854: [PATCH] Add lexical-binding cookie to autoload files

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

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

Severity: wishlist

Does anyone have any objections to, or see any problems with adding the
lexical-binding cookie to autoload files, as in the attached patch?

I've tested it, and it works just fine here.

[-- Attachment #2: 0001-Add-lexical-binding-cookie-to-autoload-files.patch --]
[-- Type: text/x-diff, Size: 999 bytes --]

From c048a7fbfea65252ff3856b69651f7b6dd0f2682 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Tue, 20 Oct 2020 16:07:20 +0200
Subject: [PATCH] Add lexical-binding cookie to autoload files

* build-aux/update-subdirs:
* lisp/emacs-lisp/autoload.el (autoload-rubric): Add lexical-binding
cookie to generated files.
---
 lisp/emacs-lisp/autoload.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/autoload.el b/lisp/emacs-lisp/autoload.el
index 07bda537b3..63c2082bbb 100644
--- a/lisp/emacs-lisp/autoload.el
+++ b/lisp/emacs-lisp/autoload.el
@@ -368,7 +368,7 @@ autoload-rubric
   (let ((basename (file-name-nondirectory file))
 	(lp (if (equal type "package") (setq type "autoloads"))))
     (concat ";;; " basename
-	    " --- automatically extracted " (or type "autoloads") "\n"
+            " --- automatically extracted " (or type "autoloads") "  -*- lexical-binding: t -*-\n"
 	    ";;\n"
 	    ";;; Code:\n\n"
 	    (if lp
-- 
2.29.2

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 24 Nov 2020 11:34:04 -0800
> Cc: monnier@iro.umontreal.ca
> 
> Does anyone have any objections to, or see any problems with adding the
> lexical-binding cookie to autoload files, as in the attached patch?

What are the benefits from doing so?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> Does anyone have any objections to, or see any problems with adding the
> lexical-binding cookie to autoload files, as in the attached patch?
> 
> I've tested it, and it works just fine here.

Ouch!

"It works just fine here."  Please define "works",
and "here".

If I understand this right then yes, I object strongly.
(It's possible I don't understand it right, however.)

1. What goes in an autoload should have nothing to do
with whether `lexical-binding' gets turned on for a
file to be autoloaded.  No?

2. The code you've changed, IIUC, is used in multiple
places, including user functions such as
`update-directory-autoloads' and
`autoload-generate-file-autoloads'.

Why on earth would we assume that a user who wants to
update her autoloads for a given user directory or
file would necessarily want to force the code to use
`lexical-binding'=t?
___

Please back off with the eagerness to paint
`lexical-binding' everywhere.  Things like this should
be handled case by case, carefully.  Spray-painting
this way is not TRT, IMO.  Sprinkling can, not fire
hose, please.  We'll get to the CL approach to lexical
& dynamic binding eventually, petit a petit.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> Does anyone have any objections to, or see any problems with adding the
>> lexical-binding cookie to autoload files, as in the attached patch?
>
> What are the benefits from doing so?

AFAICT, there are no immediate practical benefits.  But if we ever want
to have lexical-binding enabled by default, many things will have to be
done.  This is just another (admittedly small) step on that long journey.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> >> Does anyone have any objections to, or see any problems with adding
> the
> >> lexical-binding cookie to autoload files, as in the attached patch?
> >
> > What are the benefits from doing so?
> 
> AFAICT, there are no immediate practical benefits.  But if we ever want
> to have lexical-binding enabled by default, many things will have to be
> done.  This is just another (admittedly small) step on that long
> journey.

Why would we put such behavior in autoloading?

If a file is to be autoloaded then it's up to that
file whether it should have `lexical-binding' be t.

And when we get to the point that it is t by default
there will presumably be nothing to do about that wrt
autoloading.  At that point, loading a file - ANY
file, and regardless of how it is loaded (including
autoloaded) - will turn on lexical binding by default.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 24 Nov 2020 12:46:39 -0800
> Cc: 44854@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Does anyone have any objections to, or see any problems with adding the
> >> lexical-binding cookie to autoload files, as in the attached patch?
> >
> > What are the benefits from doing so?
> 
> AFAICT, there are no immediate practical benefits.  But if we ever want
> to have lexical-binding enabled by default, many things will have to be
> done.  This is just another (admittedly small) step on that long journey.

I guess I'm asking how is this a step on that journey.  Can you
elaborate?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

> If a file is to be autoloaded then it's up to that
> file whether it should have `lexical-binding' be t.

Agreed.  Does this affect that in any way?  I don't see how.

I just replied to a separate email of yours making the above point, so I
don't understand if you are simply making the same point twice, or if
this is a separate point.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Drew Adams <drew.adams@oracle.com> writes:

>> Does anyone have any objections to, or see any problems with adding the
>> lexical-binding cookie to autoload files, as in the attached patch?
>>
>> I've tested it, and it works just fine here.
>
> Ouch!
>
> "It works just fine here."  Please define "works",
> and "here".

Surely those words do not need defining.  :-)

Do you actually mean to ask how exactly I tested this?  The answer to
that is that I have used emacs with the change for a couple of hours in
my normal usage and found no issues.

> If I understand this right then yes, I object strongly.
> (It's possible I don't understand it right, however.)
[...]
> Why on earth would we assume that a user who wants to
> update her autoloads for a given user directory or
> file would necessarily want to force the code to use
> `lexical-binding'=t?

This will not force any other files to use lexical-binding.  I don't
understand what makes you think it would?  AFAIK, there is only one
thing that enables `lexical-binding' and that is if that buffer local
variable is set to t.  (You can also provide the optional LEXICAL
argument to `eval', but I don't think that applies here.)

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> > If a file is to be autoloaded then it's up to that
> > file whether it should have `lexical-binding' be t.
> 
> Agreed.  Does this affect that in any way?  I don't see how.
> 
> I just replied to a separate email of yours making the above point, so
> I don't understand if you are simply making the same point twice, or if
> this is a separate point.

I just received your other message, well after this one.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> > If I understand this right then yes, I object strongly.
> > (It's possible I don't understand it right, however.)

As I said, I may misunderstand what this change does.

I thought it was adding `lexical-binding' (by default)
to each file that gets autoloaded:

> > Why on earth would we assume that a user who wants to
> > update her autoloads for a given user directory or
> > file would necessarily want to force the code to use
> > `lexical-binding'=t?
> 
> This will not force any other files to use lexical-binding.

Good.  So what is, or are, the NON-other files where
this will force lexical-binding?

> I don't understand what makes you think it would?

Reading the code and trying to understand the patch.
I'll be glad to learn I've misunderstood, and that
this doesn't impose lexical-binding anywhere.

I don't see any necessary relation between (1)
autoload files or autoloading and (2) imposing or
declaring lexical binding by default for some files.
What am I missing?

> AFAIK, there is only one
> thing that enables `lexical-binding' and that is if that buffer local
> variable is set to t.  (You can also provide the optional LEXICAL
> argument to `eval', but I don't think that applies here.)

So what does your patch do wrt lexical binding?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I guess I'm asking how is this a step on that journey.  Can you
> elaborate?

[ In the interest of full disclosure, I had a brief private discussion
  with Stefan M about this.  I can only speak for myself, but I will
  share my clarified thinking based on his input. ]

I believe that if we want to have lexical-binding by default, we will at
some point need to begin issuing byte-compiler warnings for files that
do not explicitly say one of "-*- lexical-binding:t -*- or
"-*- lexical-binding:nil -*-".

This warning would need to be there for a period of time likely to be at
least as long as the time span we usually allow before removing any
obsolete feature.  Probably even longer.

Only after such a time period with the warning can we think about using
"lexical-binding:t" as the default.  At that point, any library that has
not yet been converted will hopefully use "lexical-binding:nil".

Now, the above idea would involve changing all of our own files to use
one of the above.  Possibly proposing this change in this manner is
putting the horse ahead of the cart.  But I was still undecided on
whether or not this was worth bringing up for general discussion on
emacs-devel for Emacs 28.

This just seemed like a small and hopefully uncontroversial step, along
the lines of: "there is no reason to not use "lexical-binding:t" in
these files, so we might as well add it now, which will make the
transition easier in the future".

I would be interested in hearing what you think about the above.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> This just seemed like a small and hopefully uncontroversial step, along
> the lines of: "there is no reason to not use "lexical-binding:t" in
> these files, so we might as well add it now, which will make the
> transition easier in the future".

What are "these files"?

My reading (possibly misreading) of the patch led me
to think that this would affect any files affected by
functions `update-directory-autoloads' and
`autoload-generate-file-autoloads', which users use
to update/specify autoloading locally.

I think you said I'm mistaken about that.  But have
you said which files _are_ affected by this?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

Stefan Kangas [2020-11-24 13:05:21] wrote:
> Drew Adams <drew.adams@oracle.com> writes:
>>Stefan Kangas wrote:
>> If a file is to be autoloaded then it's up to that
>> file whether it should have `lexical-binding' be t.

Hi Stefan,

AFAIK, this is a typical example of Drew's reaction to a suggested change:
oppose it without even the vaguest understanding of what the change
is about.

I recommend you ignore his deluge of messages which tend to just hijack
the discussion or at least derail it.


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

> Does anyone have any objections to, or see any problems with adding the
> lexical-binding cookie to autoload files, as in the attached patch?

LGTM, thanks,


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Drew Adams]

> AFAIK, this is a typical example of Drew's reaction to a suggested change: oppose it without even the vaguest understanding of what the change is about.
> 
> I recommend you ignore his deluge of messages which tend to just hijack
> the discussion or at least derail it.

(Ignoring the ad hominem attack.  Ideas please, not
people.)

I tried to understand it.  I looked at the patch and
the source code.  I asked what files this applies to.
I got no explanation of what this really does (or why).
Is it so obvious what the effect is?  It wasn't/isn't
to me.

And I said from the very beginning that I wasn't sure
I understand it, and I asked for clarification.

I responded to the question asking if anyone has a
problem with the proposed change, by saying that IF
my understanding of it is correct (and I stated my
understanding) THEN yes, I would object.

FWIW, Eli too asked what the benefits are - what this
is really for.  Haven't seen an answer to that either.

(I'm not suggesting that Eli, like me, had difficulty
understanding what this does.)

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I believe that if we want to have lexical-binding by default, we will at
> some point need to begin issuing byte-compiler warnings for files that
> do not explicitly say one of "-*- lexical-binding:t -*- or
> "-*- lexical-binding:nil -*-".

Yup.  So adding the cookie to the autoload files is a good idea -- it's
unlikely to make a difference, but if does, that's a bug that this
change will uncover.  And it's better to uncover that now, than when we
switch Emacs over to lexical binding on by default.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Tue, 24 Nov 2020 13:35:22 -0800
> Cc: 44854@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I guess I'm asking how is this a step on that journey.  Can you
> > elaborate?
> 
> [ In the interest of full disclosure, I had a brief private discussion
>   with Stefan M about this.  I can only speak for myself, but I will
>   share my clarified thinking based on his input. ]
> 
> I believe that if we want to have lexical-binding by default, we will at
> some point need to begin issuing byte-compiler warnings for files that
> do not explicitly say one of "-*- lexical-binding:t -*- or
> "-*- lexical-binding:nil -*-".
> 
> This warning would need to be there for a period of time likely to be at
> least as long as the time span we usually allow before removing any
> obsolete feature.  Probably even longer.
> 
> Only after such a time period with the warning can we think about using
> "lexical-binding:t" as the default.  At that point, any library that has
> not yet been converted will hopefully use "lexical-binding:nil".
> 
> Now, the above idea would involve changing all of our own files to use
> one of the above.  Possibly proposing this change in this manner is
> putting the horse ahead of the cart.  But I was still undecided on
> whether or not this was worth bringing up for general discussion on
> emacs-devel for Emacs 28.

See, this plan is not something that was even discussed, let alone
decided upon.  When discussing such plans in private email, please
consider the effect of that on people who didn't participate in those
discussions: they see steps being taken without the goal being clearly
announced and agreed upon.

I think before we make steps in this direction (as opposed to just
switching more and more Lisp files to lexical-binding, where there's
code that could benefit from that), we should actually discuss on
emacs-devel and agree that this is our practical intention in the near
future.

Personally, it is not clear to me that we want to switch to
lexical-binding by default in Emacs 28.  And even if we are, it is
entirely not clear that we need to mark all files with some value of
lexical-binding as a prerequisite to doing so.

> This just seemed like a small and hopefully uncontroversial step

Well, you see that it isn't.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Personally, it is not clear to me that we want to switch to
> lexical-binding by default in Emacs 28.

In Emacs 28?  I don't think that's realistic.  Perhaps in Emacs 30.

> And even if we are, it is entirely not clear that we need to mark all
> files with some value of lexical-binding as a prerequisite to doing
> so.

When we switch to lexical binding per default, the cookies can be
removed.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

> emacs-devel and agree that this is our practical intention in the near
> future.

I don't think anyone said "near future".

> Personally, it is not clear to me that we want to switch to
> lexical-binding by default in Emacs 28.

I'm pretty sure noone suggested Emacs-28 for such a switch.
Lars suggests Emacs-30, and to be honest this discussion is the first
time I see an Emacs version attached to such an idea.  I myself hadn't
even started to think about when that could happen (tho I clearly have
thought about how to get there).

The first step will be to add a warning when a file doesn't come with an
explicit `lexical-binding:` cookie.

I hadn't thought about when this could be done, but yes, I guess this
could be done in Emacs-28 already, indeed.  If so, we should start by
adding `lexical-binding:nil` cookies to all the files we haven't
converted yet.


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Stefan Kangas <stefan@marxist.se>,  44854@debbugs.gnu.org
> Date: Wed, 25 Nov 2020 11:40:03 -0500
> 
> > emacs-devel and agree that this is our practical intention in the near
> > future.
> 
> I don't think anyone said "near future".
> 
> > Personally, it is not clear to me that we want to switch to
> > lexical-binding by default in Emacs 28.
> 
> I'm pretty sure noone suggested Emacs-28 for such a switch.
> Lars suggests Emacs-30, and to be honest this discussion is the first
> time I see an Emacs version attached to such an idea.  I myself hadn't
> even started to think about when that could happen (tho I clearly have
> thought about how to get there).

Then perhaps it isn't yet time to start putting the cookies into files
that don't need it, like the autoloads files?

> The first step will be to add a warning when a file doesn't come with an
> explicit `lexical-binding:` cookie.

That's an annoyance I think we should avoid.  It shouldn't be needed.
It's like having a warning for a problem that will take a long time to
get rid of.  Bad idea, IMO.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

> Then perhaps it isn't yet time to start putting the cookies into files
> that don't need it, like the autoloads files?

I think it would be good to aim for Emacs-28 being distributed with all
*its* .el files as well as the Elisp files it generates using
`lexical-binding:t`.  That's why I'm in favor of adding this cookie to
the autoload files now.

>> The first step will be to add a warning when a file doesn't come with an
>> explicit `lexical-binding:` cookie.
> That's an annoyance I think we should avoid.  It shouldn't be needed.
> It's like having a warning for a problem that will take a long time to
> get rid of.  Bad idea, IMO.

So we should first make sure we use `lexical-binding:t` everywhere.
Fine by me,


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: stefan@marxist.se,  44854@debbugs.gnu.org
> Date: Wed, 25 Nov 2020 12:54:39 -0500
> 
> > Then perhaps it isn't yet time to start putting the cookies into files
> > that don't need it, like the autoloads files?
> 
> I think it would be good to aim for Emacs-28 being distributed with all
> *its* .el files as well as the Elisp files it generates using
> `lexical-binding:t`.  That's why I'm in favor of adding this cookie to
> the autoload files now.

What's the rationale?  Why not leave it without the cookie, right
until the time we turn lexical-binding on by default?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

> What's the rationale?  Why not leave it without the cookie, right
> until the time we turn lexical-binding on by default?

Because when we change the default (not sure what that could be, 2030
maybe?), it will change the way existing files are treated except those
that have a `lexical-binding:` cookie.  Clearly, this is dangerous and
so we want to minimize the number of those files until then.

That's the motivation also for the proposal to add a warning when the
cookie is absent.


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Philipp Stephani]

Am Mi., 25. Nov. 2020 um 19:58 Uhr schrieb Stefan Monnier
<monnier@iro.umontreal.ca>:
> That's the motivation also for the proposal to add a warning when the
> cookie is absent.

We should probably even make it an error for one major version to
force users to make a decision.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: stefan@marxist.se,  44854@debbugs.gnu.org
> Date: Wed, 25 Nov 2020 13:56:40 -0500
> 
> > What's the rationale?  Why not leave it without the cookie, right
> > until the time we turn lexical-binding on by default?
> 
> Because when we change the default (not sure what that could be, 2030
> maybe?), it will change the way existing files are treated except those
> that have a `lexical-binding:` cookie.  Clearly, this is dangerous

Dangerous for loaddefs.el? why?

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 25 Nov 2020 20:01:12 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, Stefan Kangas <stefan@marxist.se>, 44854@debbugs.gnu.org
> 
> Am Mi., 25. Nov. 2020 um 19:58 Uhr schrieb Stefan Monnier
> <monnier@iro.umontreal.ca>:
> > That's the motivation also for the proposal to add a warning when the
> > cookie is absent.
> 
> We should probably even make it an error for one major version to
> force users to make a decision.

Not a chance for that flying, sorry.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Philipp Stephani]

Am Mi., 25. Nov. 2020 um 20:13 Uhr schrieb Eli Zaretskii <eliz@gnu.org>:
>
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: stefan@marxist.se,  44854@debbugs.gnu.org
> > Date: Wed, 25 Nov 2020 13:56:40 -0500
> >
> > > What's the rationale?  Why not leave it without the cookie, right
> > > until the time we turn lexical-binding on by default?
> >
> > Because when we change the default (not sure what that could be, 2030
> > maybe?), it will change the way existing files are treated except those
> > that have a `lexical-binding:` cookie.  Clearly, this is dangerous
>
> Dangerous for loaddefs.el? why?
>

This is just basic risk management. If you want to flip a nontrivial
default, you always first switch all existing users to the new default
value explicitly.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 25 Nov 2020 20:35:54 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, Stefan Kangas <stefan@marxist.se>, 44854@debbugs.gnu.org
> 
> > > Because when we change the default (not sure what that could be, 2030
> > > maybe?), it will change the way existing files are treated except those
> > > that have a `lexical-binding:` cookie.  Clearly, this is dangerous
> >
> > Dangerous for loaddefs.el? why?
> 
> This is just basic risk management.

When there's risk, yes.  But I see no risk whatsoever in this case.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Monnier]

>> > What's the rationale?  Why not leave it without the cookie, right
>> > until the time we turn lexical-binding on by default?
>> Because when we change the default (not sure what that could be, 2030
>> maybe?), it will change the way existing files are treated except those
>> that have a `lexical-binding:` cookie.  Clearly, this is dangerous
> Dangerous for loaddefs.el? why?

I was not discussing specifically `loaddefs.el`.
The reason to change it in `loaddefs.el` now rather than later is
because I can't see any benefit to doing it later.


        Stefan

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: stefan@marxist.se,  44854@debbugs.gnu.org
> Date: Wed, 25 Nov 2020 15:06:40 -0500
> 
> >> > What's the rationale?  Why not leave it without the cookie, right
> >> > until the time we turn lexical-binding on by default?
> >> Because when we change the default (not sure what that could be, 2030
> >> maybe?), it will change the way existing files are treated except those
> >> that have a `lexical-binding:` cookie.  Clearly, this is dangerous
> > Dangerous for loaddefs.el? why?
> 
> I was not discussing specifically `loaddefs.el`.
> The reason to change it in `loaddefs.el` now rather than later is
> because I can't see any benefit to doing it later.

That's not a reason good enough in my book, sorry.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> See, this plan is not something that was even discussed, let alone
> decided upon.  When discussing such plans in private email, please
> consider the effect of that on people who didn't participate in those
> discussions: they see steps being taken without the goal being clearly
> announced and agreed upon.

I completely see your point Eli, and I understand your frustration.
Private plans could indeed play a negative role if they are not agreed
upon but implemented, especially when it comes to big issues such as
this one.

But the patch here was actually based on my impression that it was
generally agreed that we should convert files to use lexical-binding.
However, when you asked why I thought that these files should be
converted, even though there are few immediate practical benefits, I
felt that it deserved a full answer.

The other "plan" is not exactly (yet) worthy of that name to be honest.

Here's the story:

I have learned the hard way that a discussion on emacs-devel can
unfortunately very easily focus on exactly the wrong things.  I guess to
some extent that's just the result of the limitations of discussing via
email.  My intention was therefore to prepare something well-thought-out
that could hopefully constructively move things forward with
lexical-binding.

So I asked Stefan M privately what he thought about warning when
lexical-binding is not set to t.  He replied that it is a bad idea, and
that we should probably warn if it is not set to t or nil.  I agreed,
and that's pretty much it.

So the only "plan" that can be said to have existed was my personal
intention to bring it emacs-devel.  But I hadn't yet done so: I was
still mulling over the timing (before/after the relase of Emacs 28?) and
more importantly its content (i.e. I was planning to write up a patch).

I hope that clarifies things.  It was never my intention to surprise
anyone, least of all any of the maintainers.  Sorry for not being more
clear in my previous emails and having created confusion.

> I think before we make steps in this direction (as opposed to just
> switching more and more Lisp files to lexical-binding, where there's
> code that could benefit from that), we should actually discuss on
> emacs-devel and agree that this is our practical intention in the near
> future.

OK!  It sounds good to take this to emacs-devel.  Let's do it.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> I was not discussing specifically `loaddefs.el`.
>> The reason to change it in `loaddefs.el` now rather than later is
>> because I can't see any benefit to doing it later.
>
> That's not a reason good enough in my book, sorry.

FWIW, I have been working towards this crude metric:

  find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | wc -l

This gives some indicator for how ready we are to "flip the switch".  An
even better metric would of course be possible, for example by counting
SLOC or making it ignore files with side-effect free statements.  But it
hasn't seemed worth the trouble.

Also, the above metric can easily be converted to a TODO-list:

  find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | sort

This is how I've been using it.  I've been looking to make the number of
files go down by adding the cookie to even insignificant files, as they
stand in the way of seeing the files that actually need work.[1]

May I ask why you are against it?  If we agree that it should make no
difference one way or the other, why not just do it?

Footnotes:
[1] This also doubles as a hack to make this task more fun: there is
    actual science saying that human beings enjoy seeing numbers
    increase or go down.  Lars has been doing something similar with
    triaging bugs. :-)

     https://lars.ingebrigtsen.no/2020/10/26/5x10/

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Wed, 25 Nov 2020 16:07:19 -0500
> Cc: 44854@debbugs.gnu.org
> 
> >> I was not discussing specifically `loaddefs.el`.
> >> The reason to change it in `loaddefs.el` now rather than later is
> >> because I can't see any benefit to doing it later.
> >
> > That's not a reason good enough in my book, sorry.
> 
> FWIW, I have been working towards this crude metric:
> 
>   find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | wc -l
> 
> This gives some indicator for how ready we are to "flip the switch".  An
> even better metric would of course be possible, for example by counting
> SLOC or making it ignore files with side-effect free statements.  But it
> hasn't seemed worth the trouble.
> 
> Also, the above metric can easily be converted to a TODO-list:
> 
>   find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | sort
> 
> This is how I've been using it.  I've been looking to make the number of
> files go down by adding the cookie to even insignificant files, as they
> stand in the way of seeing the files that actually need work.[1]
> 
> May I ask why you are against it?  If we agree that it should make no
> difference one way or the other, why not just do it?

I have nothing against converting Lisp files to lexical-binding, and
have never objected to any of your recent changes in that direction.
But this makes sense only in files which be affected by that, and
autoloads files aren't.  "Converting" them to lexical-binding, and
modifying the code which produces those files on top of that, sounds
like we are afraid of our own shadows, or don't know what
lexical-binding is about (or both).

My point is that when the time comes to "flip the switch", we can do
that without having lexical-binding in autoloads files.  They cannot
be affected by the switch.  If they are, it's probably some subtle bug
somewhere.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Andy Moreton]

On Thu 26 Nov 2020, Eli Zaretskii wrote:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Wed, 25 Nov 2020 16:07:19 -0500
>> Cc: 44854@debbugs.gnu.org
>> 
>> >> I was not discussing specifically `loaddefs.el`.
>> >> The reason to change it in `loaddefs.el` now rather than later is
>> >> because I can't see any benefit to doing it later.
>> >
>> > That's not a reason good enough in my book, sorry.
>> 
>> FWIW, I have been working towards this crude metric:
>> 
>>   find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | wc -l
>> 
>> This gives some indicator for how ready we are to "flip the switch".  An
>> even better metric would of course be possible, for example by counting
>> SLOC or making it ignore files with side-effect free statements.  But it
>> hasn't seemed worth the trouble.
>> 
>> Also, the above metric can easily be converted to a TODO-list:
>> 
>>   find -iname "*.el" -exec egrep -L "lexical-binding: *t" {} \; | sort
>> 
>> This is how I've been using it.  I've been looking to make the number of
>> files go down by adding the cookie to even insignificant files, as they
>> stand in the way of seeing the files that actually need work.[1]
>> 
>> May I ask why you are against it?  If we agree that it should make no
>> difference one way or the other, why not just do it?
>
> I have nothing against converting Lisp files to lexical-binding, and
> have never objected to any of your recent changes in that direction.
> But this makes sense only in files which be affected by that, and
> autoloads files aren't.  "Converting" them to lexical-binding, and
> modifying the code which produces those files on top of that, sounds
> like we are afraid of our own shadows, or don't know what
> lexical-binding is about (or both).

If adding a lexical-binding tag has no effect on the autoload files
(other than making them a few bytes larger), then it should be of no
consequence. The "conversion" would be the same as for many other files
that did not require any substantive code changes.

Adding the tag to the autoloads is not about needing code changes, but
simply a marker that those files are ready for the flip-the-switch
event.

> My point is that when the time comes to "flip the switch", we can do
> that without having lexical-binding in autoloads files.  They cannot
> be affected by the switch.  If they are, it's probably some subtle bug
> somewhere.

However, by treating all .el files uniformly (including autoloads), it
makes it easier to write scripts that check for lexical binding support
in source files. Simplifying the conversion process help to ensure that
a future flip-the-switch flag day is less likely to introduce problems.

    AndyM

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Fri, 27 Nov 2020 22:39:27 +0000
> 
> If adding a lexical-binding tag has no effect on the autoload files
> (other than making them a few bytes larger), then it should be of no
> consequence.

The additional code in autoload.el is such a consequence, if nothing
else.

> Adding the tag to the autoloads is not about needing code changes, but
> simply a marker that those files are ready for the flip-the-switch
> event.

They are already ready for the flip.

> However, by treating all .el files uniformly (including autoloads), it
> makes it easier to write scripts that check for lexical binding support
> in source files.

That is an advantage too small to justify the change.  Skipping files
that all match a simple wildcard pattern is easy.  We will always need
to skip some files anyway, for example .dir-locals.el.

It makes no sense to me, sorry.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> It makes no sense to me, sorry.

It makes sense to me, both from a simplicity perspective (as a way to
keep track of which files have been converted) and from a reliability
perspective (we get testing to ensure that, indeed, there's no problem
with having these files be lexically bound).

I don't see why you object to the proposed patch.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> If adding a lexical-binding tag has no effect on the autoload files
>> (other than making them a few bytes larger), then it should be of no
>> consequence.
>
> The additional code in autoload.el is such a consequence, if nothing
> else.

It's a one line patch.

> That is an advantage too small to justify the change.  Skipping files
> that all match a simple wildcard pattern is easy.

There are only small benefits to adding this, sure.  But there seem to
be absolutely no benefits to be had from not adding it.  On the
contrary, you seem to agree that it requires us to do extra work.

bug#44854: [PATCH] Add lexical-binding cookie to autoload files

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Does anyone have any objections to, or see any problems with adding the
> lexical-binding cookie to autoload files, as in the attached patch?
>
> I've tested it, and it works just fine here.

There were objections, but two out of three maintainers were for it, so
I'm now pushing this to Emacs 28 (after checking that a "make bootstrap"
works fine, as expected).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no