[PATCH] Fix (letrec ((ignore)))

[PATCH] Fix (letrec ((ignore)))

[Zack Piper]

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

Org mode uses `(letrec ((ignore)) ...)' a few times, this introduces
compatibility after the recent setq changed which stopped an
odd number of arguments being passed.
-- 
Zack Piper <zack@apertron.net> http://apertron.net

[-- Attachment #2: 0001-Fix-letrec-and-ignore-producing-an-odd-numbered-call.patch --]
[-- Type: text/plain, Size: 1160 bytes --]

From df31b93604b28ee9b098280907fb8d5c12710b63 Mon Sep 17 00:00:00 2001
From: Zack Piper <zack.piper123@gmail.com>
Date: Fri, 11 Dec 2015 16:00:37 +0000
Subject: [PATCH] Fix `letrec' and `(ignore)' producing an odd-numbered call to
 `setq'.

Revision 5d93a89e805baa2f29941fd801e48235f6c1a6b6 introduces a change to
`setq' where it no longer accepts an odd amount of arguments. Org mode
uses `(letrec ((ignore)) ...)' a few times and other packages might do
the same.

Copyright-paperwork-exempt: yes
---
 lisp/subr.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/subr.el b/lisp/subr.el
index 860c14c..e8ba4d8 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1500,7 +1500,10 @@ letrec
   ;; making the vars actually unbound during evaluation of the binders).
   (declare (debug let) (indent 1))
   `(let ,(mapcar #'car binders)
-     ,@(mapcar (lambda (binder) `(setq ,@binder)) binders)
+     ,@(mapcar (lambda (binder)
+                 (if (second binder)
+                     `(setq ,@binder)
+                   `(setq ,@binder nil))) binders)
      ,@body))
 
 (defmacro with-wrapper-hook (hook args &rest body)
-- 
2.6.3

Re: [PATCH] Fix (letrec ((ignore)))

[Kaushal Modi]

On Fri, Dec 11, 2015 at 11:11 AM, Zack Piper <zack@apertron.net> wrote:
> Org mode uses `(letrec ((ignore)) ...)' a few times, this introduces

Would it be better instead to fix the org-mode files for consistency?

- Replace all instances of

(letrec ((SOMEVAR)) ..)

with

(letrec ((SOMEVAR nil)) ..)

Question to emacs-devel: What would be the right approach?

--
Kaushal Modi

Re: [PATCH] Fix (letrec ((ignore)))

[John Wiegley]

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

>>>>> Kaushal Modi <kaushal.modi@gmail.com> writes:

> Would it be better instead to fix the org-mode files for consistency?
> - Replace all instances of
> (letrec ((SOMEVAR)) ..)
> with
> (letrec ((SOMEVAR nil)) ..)

(letrec ((SOMEVAR))) should be an error, since the value is missing. If one
wishes to declare SOMEVAR with a nil value, the correct forms are:

    (letrec ((SOMEVAR nil)))
    (letrec (SOMEVAR))

> Question to emacs-devel: What would be the right approach?

We should give `letrec' the same treatment we've given to `setq' and `setf',
and then Org and other users should be corrected.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

Re: [PATCH] Fix (letrec ((ignore)))

[Kyle Meyer]

John Wiegley <jwiegley@gmail.com> writes:

> (letrec ((SOMEVAR))) should be an error, since the value is missing. If one
> wishes to declare SOMEVAR with a nil value, the correct forms are:
>
>     (letrec ((SOMEVAR nil)))
>     (letrec (SOMEVAR))

Doesn't the second form currently signal an error?

> and then Org and other users should be corrected.

Should be fixed in the Org repo as of f57778090.

-- 
Kyle

Re: [PATCH] Fix (letrec ((ignore)))

[Alan Mackenzie]

Hello, John.

On Sat, Dec 12, 2015 at 03:10:05PM -0800, John Wiegley wrote:
> >>>>> Kaushal Modi <kaushal.modi@gmail.com> writes:

> > Would it be better instead to fix the org-mode files for consistency?
> > - Replace all instances of
> > (letrec ((SOMEVAR)) ..)
> > with
> > (letrec ((SOMEVAR nil)) ..)

> (letrec ((SOMEVAR))) should be an error, since the value is missing. If one
> wishes to declare SOMEVAR with a nil value, the correct forms are:

>     (letrec ((SOMEVAR nil)))
>     (letrec (SOMEVAR))

> > Question to emacs-devel: What would be the right approach?

> We should give `letrec' the same treatment we've given to `setq' and `setf',
> and then Org and other users should be corrected.

Just as a matter of interest,

   (let ((foo) ... ) ...)

is accepted by the interpreter and byte compiler, binding foo to nil.

Should we make that form invalid, too?

> -- 
> John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
> http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: [PATCH] Fix (letrec ((ignore)))

[Drew Adams]

>    (let ((foo) ... ) ...)
> 
> is accepted by the interpreter and byte compiler, binding foo to nil.
> Should we make that form invalid, too?

Of course not.  It is helpful to a (human) reader to distinguish
the use of a `(foo  nil)' let-binding from a `(foo)' let-binding.

Some coders use the convention that showing the nil initial
value explicitly indicates that this is the initialization,
whereas `(foo)' indicates that the `let' body will perform
the (real, significant) initialization.

From "Common Lisp, The Language", section Establishing New
Variable Bindings:

"Instead of a list (varj valuej), one may write simply varj.
In this case varj is initialized to nil.  As a matter of style,
it is recommended that varj be written only when that variable
will be stored into (such as by setq) before its first use.
If it is important that the initial value be nil rather than
some undefined value, then it is clearer to write out (varj nil)
if the initial value is intended to mean ``false,'' or
(varj '()) if the initial value is intended to be an empty list."

RE: [PATCH] Fix (letrec ((ignore)))

[Kaushal Modi]

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

> Some coders use the convention that showing the nil initial
> value explicitly indicates that this is the initialization,
> whereas `(foo)' indicates that the `let' body will perform
> the (real, significant) initialization.

The suggestion was to consider the below form as invalid.

(letrec ((SOMEVAR1)
         (SOMEVAR2) ..) ..)

Not the below:

(letrec (SOMEVAR1
         SOMEVAR2 ..) ..)

[-- Attachment #2: Type: text/html, Size: 546 bytes --]

RE: [PATCH] Fix (letrec ((ignore)))

[Drew Adams]

> The suggestion was to consider the below form as invalid. 
> (letrec ((SOMEVAR1) 
>          (SOMEVAR2) ..) ..)
> Not the below: 
> (letrec (SOMEVAR1 SOMEVAR2 ..) ..)

Sorry; I didn't read (or write) carefully.  Please ignore.

Re: [PATCH] Fix (letrec ((ignore)))

[John Wiegley]

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

>>>>> Alan Mackenzie <acm@muc.de> writes:

> Just as a matter of interest,
>    (let ((foo) ... ) ...)
> is accepted by the interpreter and byte compiler, binding foo to nil.

In the spirit of our recent clarifications, I think it should also be made an
error, rather than an implicit binding to nil.

I have a feeling there are still many more places where this behavior exists,
and we are just beginning to scratch the surface.  For example:

    (dolist ((i)) (message "Hello"))

Is also accepted as a "do nothing" block, when it should give an error about a
missing value.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

Re: [PATCH] Fix (letrec ((ignore)))

[Alan Mackenzie]

Hello, John.

On Mon, Dec 14, 2015 at 10:23:38AM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> > Just as a matter of interest,
> >    (let ((foo) ... ) ...)
> > is accepted by the interpreter and byte compiler, binding foo to nil.

> In the spirit of our recent clarifications, I think it should also be made an
> error, rather than an implicit binding to nil.

I should have checked before writing my last contribution, but (let
((foo) ...) ...) is actually documented as permissible in the Elisp
manual.

That weakens considerably the case for making it invalid.

> I have a feeling there are still many more places where this behavior exists,
> and we are just beginning to scratch the surface.  For example:

>     (dolist ((i)) (message "Hello"))

> Is also accepted as a "do nothing" block, when it should give an error about a
> missing value.

Outch, that's confusing.

> -- 
> John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
> http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: [PATCH] Fix (letrec ((ignore)))

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

> I should have checked before writing my last contribution, but (let ((foo)
> ...) ...) is actually documented as permissible in the Elisp manual.

> That weakens considerably the case for making it invalid.

Perhaps this is a bug both in the code and the manual?  What say others?

>> Is also accepted as a "do nothing" block, when it should give an error
>> about a missing value.

> Outch, that's confusing.

Yes, I'd like to achieve consistency here, if it's not going to harm things.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH] Fix (letrec ((ignore)))

[David Kastrup]

John Wiegley <jwiegley@gmail.com> writes:

>>>>>> Alan Mackenzie <acm@muc.de> writes:
>
>> I should have checked before writing my last contribution, but (let ((foo)
>> ...) ...) is actually documented as permissible in the Elisp manual.
>
>> That weakens considerably the case for making it invalid.
>
> Perhaps this is a bug both in the code and the manual?  What say others?

It's awful as far as I'm concerned.  But being documented and available
for a long time, deprecating it is a slow process, starting with
undocumenting it and giving byte compiler warnings.

I'd not mind getting it on the way.

-- 
David Kastrup

Re: [PATCH] Fix (letrec ((ignore)))

[John Wiegley]

>>>>> David Kastrup <dak@gnu.org> writes:

> It's awful as far as I'm concerned. But being documented and available for a
> long time, deprecating it is a slow process, starting with undocumenting it
> and giving byte compiler warnings.

We haven't taken that approach with setq/setf, but I'm fine either way. I'm
also willing to leave this one alone until it starts biting someone, since I
imagine letrec is far less commonly used.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [PATCH] Fix (letrec ((ignore)))

[Alan Mackenzie]

Hello, David.

On Mon, Dec 14, 2015 at 09:47:49PM +0100, David Kastrup wrote:
> John Wiegley <jwiegley@gmail.com> writes:

> >>>>>> Alan Mackenzie <acm@muc.de> writes:

> >> I should have checked before writing my last contribution, but (let ((foo)
> >> ...) ...) is actually documented as permissible in the Elisp manual.

> >> That weakens considerably the case for making it invalid.

> > Perhaps this is a bug both in the code and the manual?  What say others?

> It's awful as far as I'm concerned.  But being documented and available
> for a long time, deprecating it is a slow process, starting with
> undocumenting it and giving byte compiler warnings.

I've just tried putting a diagnostic into the compiler.  A make
bootstrap turned up 225 occurences of ((foo)) style bindings.

As you say, getting rid of them will be a slow process.  :-(

> I'd not mind getting it on the way.

> -- 
> David Kastrup

-- 
Alan Mackenzie (Nuremberg, Germany).