bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

Basically, if an emacs lisp source file starts with some whitespace

| ;; -*- lexical-binding: t -*-
|(let ((x 1)) (setq foo (lambda () x)))
|(funcall foo)

rather than

|;; -*- lexical-binding: t -*-
|(let ((x 1)) (setq foo (lambda () x)))
|(funcall foo)

that will cause `load' eval the file with `lexical-binding' set to nil,
and would report `x' is a void variable.

This behavior is in contrast to how file local variables are applied
when opening a file, that is, as long as the variable list is in
the first line, it is applied.

Either the documentation should bring up this behavior, or the
C function `lisp_file_lexically_bound_p' in `src/lread.c' should be fixed.



-- 
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> Date: Sat, 24 Jun 2023 13:22:38 -0500
> From: LdBeth <andpuke@foxmail.com>
> 
> 
> Basically, if an emacs lisp source file starts with some whitespace
> 
> | ;; -*- lexical-binding: t -*-
> |(let ((x 1)) (setq foo (lambda () x)))
> |(funcall foo)
> 
> rather than
> 
> |;; -*- lexical-binding: t -*-
> |(let ((x 1)) (setq foo (lambda () x)))
> |(funcall foo)
> 
> that will cause `load' eval the file with `lexical-binding' set to nil,
> and would report `x' is a void variable.
> 
> This behavior is in contrast to how file local variables are applied
> when opening a file, that is, as long as the variable list is in
> the first line, it is applied.
> 
> Either the documentation should bring up this behavior, or the
> C function `lisp_file_lexically_bound_p' in `src/lread.c' should be fixed.

I think we should do the latter, because
hack-local-variables-prop-line is more lenient than
lisp_file_lexically_bound_p.

Stefan, any comments?

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

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

>> Either the documentation should bring up this behavior, or the
>> C function `lisp_file_lexically_bound_p' in `src/lread.c' should be fixed.
> I think we should do the latter, because
> hack-local-variables-prop-line is more lenient than
> lisp_file_lexically_bound_p.

I'd tend to agree.


        Stefan "Looking forward to the day we can default
                `lexical-binding` to non-nil"

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

>>>>> In <83zg4oy9ow.fsf@gnu.org> 
>>>>>	Eli Zaretskii <eliz@gnu.org> wrote:

ldb> This behavior is in contrast to how file local variables are applied
ldb> when opening a file, that is, as long as the variable list is in
ldb> the first line, it is applied.
ldb> 
ldb> Either the documentation should bring up this behavior, or the
ldb> C function `lisp_file_lexically_bound_p' in `src/lread.c' should be fixed.

Eli> I think we should do the latter, because
Eli> hack-local-variables-prop-line is more lenient than
Eli> lisp_file_lexically_bound_p.

Eli> Stefan, any comments?

If so, I think the only thing needs to be changed is
remove the `if (ch != ';')` test.

Since from what I test, even if first line is something like

|(setq tmp " -*- lexical-binding: t -*- ")

the file local variables would still be applied.

---
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> Date: Sat, 24 Jun 2023 21:11:11 -0500
> From: LdBeth <andpuke@foxmail.com>
> Cc: LdBeth <andpuke@foxmail.com>,
> 	Stefan Monnier <monnier@iro.umontreal.ca>,
> 	64272@debbugs.gnu.org
> 
> Eli> I think we should do the latter, because
> Eli> hack-local-variables-prop-line is more lenient than
> Eli> lisp_file_lexically_bound_p.
> 
> Eli> Stefan, any comments?
> 
> If so, I think the only thing needs to be changed is
> remove the `if (ch != ';')` test.
> 
> Since from what I test, even if first line is something like
> 
> |(setq tmp " -*- lexical-binding: t -*- ")
> 
> the file local variables would still be applied.

I'm not sure we want to support this outside of a Lisp comment.
Stefan, WDYT?  Could false positives cause harm?

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

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

> I'm not sure we want to support this outside of a Lisp comment.
> Stefan, WDYT?  Could false positives cause harm?

I'd much rather we try and stay as close as possible to the behavior of
`hack-local-variables-prop-line`


        Stefan

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: LdBeth <andpuke@foxmail.com>,  64272@debbugs.gnu.org
> Date: Sun, 25 Jun 2023 09:37:31 -0400
> 
> > I'm not sure we want to support this outside of a Lisp comment.
> > Stefan, WDYT?  Could false positives cause harm?
> 
> I'd much rather we try and stay as close as possible to the behavior of
> `hack-local-variables-prop-line`

Right, but that doesn't require the -*- line to be a comment:

      (goto-char (point-min))
      (let ((end (set-auto-mode-1))
	    result)
	(cond ((not end)
	       nil)
	      ((looking-at "[ \t]*\\([^ \t\n\r:;]+\\)\\([ \t]*-\\*-\\)")
	       ;; Simple form: "-*- MODENAME -*-".
	       (if (eq handle-mode t)
		   (intern (concat (match-string 1) "-mode"))))
	      (t
	       ;; Hairy form: '-*-' [ <variable> ':' <value> ';' ]* '-*-'
	       ;; (last ";" is optional).
	       ;; If HANDLE-MODE is t, just check for `mode'.
	       ;; Otherwise, parse the -*- line into the RESULT alist.

The fact that most -*- lines are in comments is because they are in
program source files, so we need to hide them from the compiler or the
interpreter.

Am I missing something?

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

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

>> I'd much rather we try and stay as close as possible to the behavior of
>> `hack-local-variables-prop-line`
>
> Right, but that doesn't require the -*- line to be a comment:
>
>       (goto-char (point-min))
>       (let ((end (set-auto-mode-1))
> 	    result)
> 	(cond ((not end)
> 	       nil)
> 	      ((looking-at "[ \t]*\\([^ \t\n\r:;]+\\)\\([ \t]*-\\*-\\)")
> 	       ;; Simple form: "-*- MODENAME -*-".
> 	       (if (eq handle-mode t)
> 		   (intern (concat (match-string 1) "-mode"))))
> 	      (t
> 	       ;; Hairy form: '-*-' [ <variable> ':' <value> ';' ]* '-*-'
> 	       ;; (last ";" is optional).
> 	       ;; If HANDLE-MODE is t, just check for `mode'.
> 	       ;; Otherwise, parse the -*- line into the RESULT alist.
>
> The fact that most -*- lines are in comments is because they are in
> program source files, so we need to hide them from the compiler or the
> interpreter.

Oh, indeed.  Then `ldb`s suggestion sounds great since it makes us "stay
as close as possible to the behavior of
`hack-local-variables-prop-line`".


        Stefan

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

>>>>> In <83mt0ny47z.fsf@gnu.org> 
>>>>>	Eli Zaretskii <eliz@gnu.org> wrote:

Eli> I'm not sure we want to support this outside of a Lisp comment.
Eli> Stefan, WDYT?  Could false positives cause harm?
 
Stefan> I'd much rather we try and stay as close as possible to the behavior of
Stefan> `hack-local-variables-prop-line`

Eli> The fact that most -*- lines are in comments is because they are in
Eli> program source files, so we need to hide them from the compiler or the
Eli> interpreter.

Eli> Am I missing something?

Ok I find the reason the `lisp_file_lexically_bound_p' would give
up if the first line isn't a comment is, it said:

 Return true if the lisp code read using READCHARFUN defines a non-nil
 `lexical-binding' file variable.  After returning, the stream is
 positioned following the first line, if it is a comment or #! line,
 otherwise nothing is read.

So this function assumes the first line is discarded if file local
variables would have been read.

If first line is lisp code, and we want to keep the similar
behavior of `hack-local-variables-prop-line`, there needs a mean
to buffer the content of the first line. (Or reset file position
but I don't think there is a way to do that without
substantially change lread.c)

But it would be easier to only handle the extra whitespace at
beginning of file.

---
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> Date: Sun, 25 Jun 2023 12:17:40 -0500
> From: LdBeth <andpuke@foxmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
> 	andpuke@foxmail.com,
> 	64272@debbugs.gnu.org
> 
> >>>>> In <83mt0ny47z.fsf@gnu.org> 
> >>>>>	Eli Zaretskii <eliz@gnu.org> wrote:
> 
> Eli> I'm not sure we want to support this outside of a Lisp comment.
> Eli> Stefan, WDYT?  Could false positives cause harm?
>  
> Stefan> I'd much rather we try and stay as close as possible to the behavior of
> Stefan> `hack-local-variables-prop-line`
> 
> Eli> The fact that most -*- lines are in comments is because they are in
> Eli> program source files, so we need to hide them from the compiler or the
> Eli> interpreter.
> 
> Eli> Am I missing something?
> 
> Ok I find the reason the `lisp_file_lexically_bound_p' would give
> up if the first line isn't a comment is, it said:
> 
>  Return true if the lisp code read using READCHARFUN defines a non-nil
>  `lexical-binding' file variable.  After returning, the stream is
>  positioned following the first line, if it is a comment or #! line,
>  otherwise nothing is read.
> 
> So this function assumes the first line is discarded if file local
> variables would have been read.
> 
> If first line is lisp code, and we want to keep the similar
> behavior of `hack-local-variables-prop-line`, there needs a mean
> to buffer the content of the first line. (Or reset file position
> but I don't think there is a way to do that without
> substantially change lread.c)

There isn't.  We can only unread one character at a time.

> But it would be easier to only handle the extra whitespace at
> beginning of file.

So now let me turn the table and ask: if we are only going to support
whitespace before the semicolon, then what exactly are we gaining
here?

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

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

> So now let me turn the table and ask: if we are only going to support
> whitespace before the semicolon, then what exactly are we gaining
> here?

I think the main problem is the disconnect between the various ways the
cookie is recognized, so it may appear to work on one end and fail to
work on the other, leaving the user rather perplexed.

So maybe we should export `lisp_file_lexically_bound_p` to ELisp, let
the byte-compiler (and maybe also `emacs-lisp-mode`) verify that it
gives the same answer as `hack-local-variables-prop-line`, and emit
a loud warning if it doesn't.


        Stefan

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

>>>>> In <83h6qvxv8q.fsf@gnu.org> 
>>>>>	Eli Zaretskii <eliz@gnu.org> wrote:
ldb> If first line is lisp code, and we want to keep the similar
ldb> behavior of `hack-local-variables-prop-line`, there needs a mean
ldb> to buffer the content of the first line. (Or reset file position
ldb> but I don't think there is a way to do that without
ldb> substantially change lread.c)

Eli> There isn't.  We can only unread one character at a time.

ldb> But it would be easier to only handle the extra whitespace at
ldb> beginning of file.

Eli> So now let me turn the table and ask: if we are only going to support
Eli> whitespace before the semicolon, then what exactly are we gaining
Eli> here?

Haha, we would gain pretty much nothing useful if the issue not resolved.


So as I continue digging into lread.c and trying to find alternative
solutions, I find these:

  DEFSYM (Qget_file_char, "get-file-char");

  /* Used instead of Qget_file_char while loading *.elc files compiled
     by Emacs 21 or older.  */
  DEFSYM (Qget_emacs_mule_file_char, "get-emacs-mule-file-char");

While `get-file-char` is exposed to emacs lisp,
`get-emacs-mule-file-char' is not even a defined lisp function.

There are multiple places in `lread.c` that handles
`Qget_emacs_mule_file_char`. Which I believe it time to consider
them as dead code and remove them.

For the only two functions that calls `lisp_file_lexically_bound_p`,
`load` is hard coded to use `get-file-char` which is a wrapper around
`getc()`, and `eval-buffer` uses the `BUFFERP (readcharfun)`
branch in `readchar`. I think both case can be changed to
use a more flexible way to test file local variables
rather than stick to the READCHAR UNREAD api.

---
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

>>>>> In <tencent_801CA17A56314C746AAC30FFB8DC58BB1109@qq.com> 
>>>>>	LdBeth <andpuke@foxmail.com> wrote:

ldb> So as I continue digging into lread.c and trying to find alternative
ldb> solutions, I find these:

ldb>   DEFSYM (Qget_file_char, "get-file-char");

ldb>   /* Used instead of Qget_file_char while loading *.elc files compiled
ldb>      by Emacs 21 or older.  */
ldb>   DEFSYM (Qget_emacs_mule_file_char, "get-emacs-mule-file-char");

ldb> While `get-file-char` is exposed to emacs lisp,
ldb> `get-emacs-mule-file-char' is not even a defined lisp function.

ldb> There are multiple places in `lread.c` that handles
ldb> `Qget_emacs_mule_file_char`. Which I believe it time to consider
ldb> them as dead code and remove them.

Sorry, I misread on how `Qget_emacs_mule_file_char' is been used.
It is still been used internally in `lread.c' because of how READCHAR
works.

ldb> For the only two functions that calls `lisp_file_lexically_bound_p`,
ldb> `load` is hard coded to use `get-file-char` which is a wrapper around
ldb> `getc()`, and `eval-buffer` uses the `BUFFERP (readcharfun)`
ldb> branch in `readchar`. I think both case can be changed to
ldb> use a more flexible way to test file local variables
ldb> rather than stick to the READCHAR UNREAD api.

I think this still holds valid.

---
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[LdBeth]

>>>>> In <jwvleg7m9ot.fsf-monnier+emacs@gnu.org> 
>>>>>	Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> So maybe we should export `lisp_file_lexically_bound_p` to
> ELisp, let the byte-compiler (and maybe also
> `emacs-lisp-mode`) verify that it gives the same answer as
> `hack-local-variables-prop-line`, and emit a loud warning if
> it doesn't.

I think it is a good idea to have byte-compiler check that.

The context of how this bug reveal is a newbie user
asked why the lexical-binding does not apply in the .emacs
file and turns out there is a leading space in the first
line of the .emacs (well to be honest only scheme people
does wired things with letrec in their .emacs file)

In the context of sharing .emacs files, the files are not likely to be
compiled, so it is better also to check that in emacs-lisp-mode, like,
when the file is opened.

---
ldb

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: LdBeth <andpuke@foxmail.com>,  64272@debbugs.gnu.org
> Date: Sun, 25 Jun 2023 19:00:04 -0400
> 
> > So now let me turn the table and ask: if we are only going to support
> > whitespace before the semicolon, then what exactly are we gaining
> > here?
> 
> I think the main problem is the disconnect between the various ways the
> cookie is recognized, so it may appear to work on one end and fail to
> work on the other, leaving the user rather perplexed.
> 
> So maybe we should export `lisp_file_lexically_bound_p` to ELisp, let
> the byte-compiler (and maybe also `emacs-lisp-mode`) verify that it
> gives the same answer as `hack-local-variables-prop-line`, and emit
> a loud warning if it doesn't.

Patches welcome.

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> Date: Sun, 25 Jun 2023 19:53:09 -0500
> From: LdBeth <andpuke@foxmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,
> 	LdBeth <andpuke@foxmail.com>,
> 	64272@debbugs.gnu.org
> 
> >>>>> In <jwvleg7m9ot.fsf-monnier+emacs@gnu.org> 
> >>>>>	Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> > So maybe we should export `lisp_file_lexically_bound_p` to
> > ELisp, let the byte-compiler (and maybe also
> > `emacs-lisp-mode`) verify that it gives the same answer as
> > `hack-local-variables-prop-line`, and emit a loud warning if
> > it doesn't.
> 
> I think it is a good idea to have byte-compiler check that.
> 
> The context of how this bug reveal is a newbie user
> asked why the lexical-binding does not apply in the .emacs
> file and turns out there is a leading space in the first
> line of the .emacs (well to be honest only scheme people
> does wired things with letrec in their .emacs file)
> 
> In the context of sharing .emacs files, the files are not likely to be
> compiled, so it is better also to check that in emacs-lisp-mode, like,
> when the file is opened.

I'm sorry to bust this particular wedding, but here we see once again
what became a frequent pattern in Emacs: a tiny problem, perhaps
affecting one or a few users who happen to bump into it the first time
they try something, which then causes quite serious changes to
low-level infrastructure in Emacs that has been working flawlessly for
years, and by doing that destabilize Emacs for no good reason.  IOW, a
tail that wags the dog.

Happens time and again in Emacs, and we all fail to pay attention.

The right solution to such problems is for users to learn not to do
that.  Which they already did learn, the first time the bumped into
this.  So the problem, such as it is, is already solved, and we still
contemplate it.

That said, I know I will be downvoted, and so will not object any
changes in this wrong direction.  Let the resultant breakage speak for
itself.

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Gregory Heytings]

>
> I'm sorry to bust this particular wedding, but here we see once again 
> what became a frequent pattern in Emacs: a tiny problem, perhaps 
> affecting one or a few users who happen to bump into it the first time 
> they try something, which then causes quite serious changes to low-level 
> infrastructure in Emacs that has been working flawlessly for years, and 
> by doing that destabilize Emacs for no good reason.  IOW, a tail that 
> wags the dog.
>
> Happens time and again in Emacs, and we all fail to pay attention.
>
> [...]
>
> That said, I know I will be downvoted, and so will not object any 
> changes in this wrong direction.  Let the resultant breakage speak for 
> itself.
>

Not that my opinion counts, but FWIW, I agree with the above, and I upvote 
this.

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Eli Zaretskii]

> Date: Mon, 26 Jun 2023 15:18:34 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: LdBeth <andpuke@foxmail.com>, 64272@debbugs.gnu.org, 
>     monnier@iro.umontreal.ca
> 
> > I'm sorry to bust this particular wedding, but here we see once again 
> > what became a frequent pattern in Emacs: a tiny problem, perhaps 
> > affecting one or a few users who happen to bump into it the first time 
> > they try something, which then causes quite serious changes to low-level 
> > infrastructure in Emacs that has been working flawlessly for years, and 
> > by doing that destabilize Emacs for no good reason.  IOW, a tail that 
> > wags the dog.
> >
> > Happens time and again in Emacs, and we all fail to pay attention.
> >
> > [...]
> >
> > That said, I know I will be downvoted, and so will not object any 
> > changes in this wrong direction.  Let the resultant breakage speak for 
> > itself.
> >
> 
> Not that my opinion counts, but FWIW, I agree with the above, and I upvote 
> this.

Thank you!  (And your opinion does count, at least here.)

bug#64272: 28.1; lisp_file_lexically_bound_p behavior mismatches file local variables

[Gerd Möllmann]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> I'm sorry to bust this particular wedding, but here we see once
>> again what became a frequent pattern in Emacs: a tiny problem,
>> perhaps affecting one or a few users who happen to bump into it the
>> first time they try something, which then causes quite serious
>> changes to low-level infrastructure in Emacs that has been working
>> flawlessly for years, and by doing that destabilize Emacs for no
>> good reason.  IOW, a tail that wags the dog.
>>
>> Happens time and again in Emacs, and we all fail to pay attention.
>>
>> [...]
>>
>> That said, I know I will be downvoted, and so will not object any
>> changes in this wrong direction.  Let the resultant breakage speak
>> for itself.
>>
>
> Not that my opinion counts, but FWIW, I agree with the above, and I
> upvote this.

+1