setq's with missing final arguments.

setq's with missing final arguments.

[Alan Mackenzie]

Hello, Emacs.

Consider this file:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(defvar foo t)
(defvar bar t)

(defun bad-setq ()
  "Doc string"
  (setq foo 5
	bar))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

In the setq, there is a missing argument after "bar".  At the moment,
the byte compiler just generates code to assign nil to bar, without
giving any warning.  IMAO, this is Very Bad.

I propose to insert code into the byte compiler to detect and warn of
this scenario:



diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 3574364..1e75d48 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -3739,7 +3739,11 @@ byte-compile-setq
   (let ((args (cdr form)))
     (if args
 	(while args
-	  (byte-compile-form (car (cdr args)))
+	  (if (eq (length args) 1)
+              (byte-compile-warn
+               "implicit nil assignment to `%s'"
+               (prin1-to-string (car args))))
+          (byte-compile-form (car (cdr args)))
 	  (or byte-compile--for-effect (cdr (cdr args))
 	      (byte-compile-out 'byte-dup 0))
 	  (byte-compile-variable-set (car args))


Any objections?

Just as a matter of interest, I came across an actual instance of this
solecism in Emacs, where it is clearly an error.  For some reason, this
instance doesn't trigger an "implicit nil assignment" warning; perhaps
it is because it is inside a pcase.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: setq's with missing final arguments.

[David Kastrup]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Emacs.
>
> Consider this file:
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (defvar foo t)
> (defvar bar t)
>
> (defun bad-setq ()
>   "Doc string"
>   (setq foo 5
> 	bar))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
> In the setq, there is a missing argument after "bar".  At the moment,
> the byte compiler just generates code to assign nil to bar, without
> giving any warning.  IMAO, this is Very Bad.
>
> I propose to insert code into the byte compiler to detect and warn of
> this scenario:

[...]

> Any objections?

Yes.  This is not a case for a warning.  It's an error.  It does not
match the documented definition of setq.

-- 
David Kastrup

Re: setq's with missing final arguments.

[Oleh Krehel]

Alan Mackenzie <acm@muc.de> writes:

> Consider this file:
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (defvar foo t)
> (defvar bar t)
>
> (defun bad-setq ()
>   "Doc string"
>   (setq foo 5
> 	bar))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
> In the setq, there is a missing argument after "bar".  At the moment,
> the byte compiler just generates code to assign nil to bar, without
> giving any warning.  IMAO, this is Very Bad.

In my opinion, multi-variable setq should be deprecated altogether.  I'm
sure many people will disagree for the reasons of habit, but
multi-variable setq is just plain bad: it makes LISP less lispy that it
should be. For example: "(setq bar)" is a nice sexp: you can delete it,
copy it, comment it, move it around - all kinds of fast, productive and
clear to see things. On the other hand, "bar" is simply a part of
another expression, you can do way less things with it, and you can
break things in bad ways if you're not careful. It's an unnecessary
mental burden on the programmer.

Just my two cents. I understand very well there's only 0.1% chance to
get this behavior deprecated because reasons. But a man can dream.  And
at least I can enforce this restriction on the contributions to my
packages.

    Oleh

Re: setq's with missing final arguments.

[Teemu Likonen]

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

Oleh Krehel [2015-11-22 13:35:45+01] wrote:

> In my opinion, multi-variable setq should be deprecated altogether.
> I'm sure many people will disagree for the reasons of habit,

And maybe also because of Common Lisp:

    Syntax:
    setq {pair}* => result
    pair::= var form 

http://www.lispworks.com/documentation/HyperSpec/Body/s_setq.htm

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

multi-assignment setq [was: setq's with missing final arguments.]

[Drew Adams]

> In my opinion, multi-variable setq should be deprecated altogether.

(This is a change of topic.  The Subject line should change too.)

Presumably, since `setq' can make multiple assignments to the
same variable, you mean not "multi-variable" but "multi-assignment".

> I'm sure many people will disagree for the reasons of habit,

I disagree completely.  But not out of habit.  In fact, for
years (quite a long time, in fact) I avoided it, in favor of
multiple `setq' calls.  If necessary, I can give the reasons
why I now prefer to use a single `setq' with multiple assignments.

> but multi-variable setq is just plain bad: it makes LISP less
> lispy that it should be. For example: "(setq bar)" is a nice
> sexp: you can delete it, copy it, comment it, move it around

That quality is not what I consider "lispiness".  That same
complaint applies to `let*' and a zillion other Lisp veterans.

Re: multi-assignment setq [was: setq's with missing final arguments.]

[Oleh Krehel]

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

>> but multi-variable setq is just plain bad: it makes LISP less
>> lispy that it should be. For example: "(setq bar)" is a nice
>> sexp: you can delete it, copy it, comment it, move it around
>
> That quality is not what I consider "lispiness".  That same
> complaint applies to `let*' and a zillion other Lisp veterans.

By "lispiness" I mean the sexp structure that allows you to manipulate
code in a foolproof way and navigate it easier. That sexp structure also
applies to `let*', but doesn't apply for example to Clojure's `let'.

For example of being foolproof, exchanging the order of two `setq'
statements is a well defined, simple to automate operation. You can pull
it off with `transpose-sexps' if you position the point just
right. Alternatively, `kill-sexp' when positioned before the open paren
can also work for this.

On the other hand, multi-assignment setq isn't foolproof. You have to
count from sexp's start and make sure that you kill/paste starting from
the odd numbered list child and not from the even numbered. It's a
hassle that can easily be avoided. Using single-assignment setq adds
only 6 chars per assignment, compared to multi-assignment setq and:

- it reads better
- it navigates better (`forward-list' and `backward-list')
- it manipulates better

RE: multi-assignment setq [was: setq's with missing final arguments.]

[Drew Adams]

> - it reads better
> - it navigates better (`forward-list' and `backward-list')
> - it manipulates better

I disagree, especially for "reads better" (which is by far the
most important, both for author and other readers).  But maybe
you like adding `progn' everywhere.  Eye of the beholder...

The important thing is that you can use your "lispier" style,
and I can use my "lispier" style.  Nothing prevents you from
writing:

  (progn (setq foo       bar)
         (setq tatasaba  tutuwimbo)
         (setq toto      titi))

if such is your wont.  Just as nothing prevents me from writing:

  (setq foo       bar
        tatasaba  tutuwimbo
        toto      titi)

to get the same effect.

There is zero reason to not allow `setq' to make multiple
assignments.  That's the point.

Re: setq's with missing final arguments.

[Andreas Schwab]

Alan Mackenzie <acm@muc.de> writes:

> Hello, Emacs.
>
> Consider this file:
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (defvar foo t)
> (defvar bar t)
>
> (defun bad-setq ()
>   "Doc string"
>   (setq foo 5
> 	bar))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
> In the setq, there is a missing argument after "bar".  At the moment,
> the byte compiler just generates code to assign nil to bar, without
> giving any warning.  IMAO, this is Very Bad.

But consistent with how setq works.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: setq's with missing final arguments.

[Artur Malabarba]

Alan said:
> In the setq, there is a missing argument after "bar".  At the moment,
the byte compiler just generates code to assign nil to bar, without
giving any warning.  IMAO, this is Very Bad.

Oleh said:
> In my opinion, multi-variable setq should be deprecated altogether.

I'm in favor of giving warnings about any one of these (we shouldn't
actually change any behavior right now, of course).
I agree with Alan that the current behavior (allowing multiple args
where the last one might be assumed to be nil) is bad (regardless of
how consistent it is), and I think both options are a fine way to warn
users.

Re: setq's with missing final arguments.

[David Kastrup]

Andreas Schwab <schwab@linux-m68k.org> writes:

> Alan Mackenzie <acm@muc.de> writes:
>
>> Hello, Emacs.
>>
>> Consider this file:
>>
>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>> (defvar foo t)
>> (defvar bar t)
>>
>> (defun bad-setq ()
>>   "Doc string"
>>   (setq foo 5
>> 	bar))
>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>
>> In the setq, there is a missing argument after "bar".  At the moment,
>> the byte compiler just generates code to assign nil to bar, without
>> giving any warning.  IMAO, this is Very Bad.
>
> But consistent with how setq works.

How so?

    setq is a special form in ‘C source code’.

    (setq [SYM VAL]...)

    Set each SYM to the value of its VAL.
    The symbols SYM are variables; they are literal (not evaluated).
    The values VAL are expressions; they are evaluated.
    Thus, (setq x (1+ y)) sets ‘x’ to the value of ‘(1+ y)’.
    The second VAL is not computed until after the first SYM is set, and so on;
    each VAL can use the new value of variables set earlier in the ‘setq’.
    The return value of the ‘setq’ form is the value of the last VAL.

    [back]

There is nothing in here whatsoever that would suggest an odd number of
arguments is allowed or does anything useful.

-- 
David Kastrup

Re: setq's with missing final arguments.

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Alan Mackenzie <acm@muc.de> writes:
>>
>>> Hello, Emacs.
>>>
>>> Consider this file:
>>>
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>> (defvar foo t)
>>> (defvar bar t)
>>>
>>> (defun bad-setq ()
>>>   "Doc string"
>>>   (setq foo 5
>>> 	bar))
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>
>>> In the setq, there is a missing argument after "bar".  At the moment,
>>> the byte compiler just generates code to assign nil to bar, without
>>> giving any warning.  IMAO, this is Very Bad.
>>
>> But consistent with how setq works.
>
> How so?

Where did I say "documented"?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: setq's with missing final arguments.

[David Kastrup]

Andreas Schwab <schwab@linux-m68k.org> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Andreas Schwab <schwab@linux-m68k.org> writes:
>>
>>> Alan Mackenzie <acm@muc.de> writes:
>>>
>>>> Hello, Emacs.
>>>>
>>>> Consider this file:
>>>>
>>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>> (defvar foo t)
>>>> (defvar bar t)
>>>>
>>>> (defun bad-setq ()
>>>>   "Doc string"
>>>>   (setq foo 5
>>>> 	bar))
>>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>>
>>>> In the setq, there is a missing argument after "bar".  At the moment,
>>>> the byte compiler just generates code to assign nil to bar, without
>>>> giving any warning.  IMAO, this is Very Bad.
>>>
>>> But consistent with how setq works.
>>
>> How so?
>
> Where did I say "documented"?

I don't see the point in pointing out that the bad actual behavior Alan
points out is consistent with the bad actual behavior Alan points out.

Presumably you mean that this afflicts both byte compiler and
interpreter.  But that's really beside the point and does not magically
make it a good idea.

-- 
David Kastrup

Re: setq's with missing final arguments.

[Andreas Schwab]

David Kastrup <dak@gnu.org> writes:

> Presumably you mean that this afflicts both byte compiler and
> interpreter.  But that's really beside the point and does not magically
> make it a good idea.

The byte-compiler is not for imposing coding style.  If it is wrong for
setq to accept implit nil then it should be changed in setq and only
then in the byte-compiler.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: setq's with missing final arguments.

[Alan Mackenzie]

Hello, Andreas.

On Sun, Nov 22, 2015 at 02:34:51PM +0100, Andreas Schwab wrote:
> David Kastrup <dak@gnu.org> writes:

> > Presumably you mean that this afflicts both byte compiler and
> > interpreter.  But that's really beside the point and does not magically
> > make it a good idea.

> The byte-compiler is not for imposing coding style.

That is why I proposed issuing a warning, not an error.  In the actual
case I found, it was indeed an error.  

> If it is wrong for setq to accept implit nil then it should be changed
> in setq and only then in the byte-compiler.

I would be happy enough for a missing final argument to setq to be an
error.  But until that stage is reached, I think issuing a warning is
appropriate.  But I disagree with you about priorities: it is more
important to catch the violations in our compiled code than worry about
the interpreted version.

> Andreas.

> -- 
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: setq's with missing final arguments.

[Eli Zaretskii]

> Date: Sun, 22 Nov 2015 14:10:31 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: David Kastrup <dak@gnu.org>, emacs-devel@gnu.org
> 
> > The byte-compiler is not for imposing coding style.
> 
> That is why I proposed issuing a warning, not an error.  In the actual
> case I found, it was indeed an error.  
> 
> > If it is wrong for setq to accept implit nil then it should be changed
> > in setq and only then in the byte-compiler.
> 
> I would be happy enough for a missing final argument to setq to be an
> error.  But until that stage is reached, I think issuing a warning is
> appropriate.  But I disagree with you about priorities: it is more
> important to catch the violations in our compiled code than worry about
> the interpreted version.

As long as 'setq' accepts this use case, it is not a violation.

Re: setq's with missing final arguments.

[Alan Mackenzie]

Hello, Eli.

On Sun, Nov 22, 2015 at 05:44:55PM +0200, Eli Zaretskii wrote:
> > Date: Sun, 22 Nov 2015 14:10:31 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: David Kastrup <dak@gnu.org>, emacs-devel@gnu.org

> > > The byte-compiler is not for imposing coding style.

> > That is why I proposed issuing a warning, not an error.  In the actual
> > case I found, it was indeed an error.  

> > > If it is wrong for setq to accept implit nil then it should be changed
> > > in setq and only then in the byte-compiler.

> > I would be happy enough for a missing final argument to setq to be an
> > error.  But until that stage is reached, I think issuing a warning is
> > appropriate.  But I disagree with you about priorities: it is more
> > important to catch the violations in our compiled code than worry about
> > the interpreted version.

> As long as 'setq' accepts this use case, it is not a violation.

It is a violation if it conceals errors.  This indeed is the case: in
.../lisp/emacs-lisp/bytecomp.el, in defun
byte-compile-file-form-autoload the following erroneous code exists:

         (setq byte-compile-noruntime-functions
               (delq funsym byte-compile-noruntime-functions)
               byte-compile-noruntime-functions)

.  The effect is to set byte-compile-noruntime-functions unconditionally
to nil.  The effect of that is to conceal some "function not defined \(at
runtime\)?" warnings in source files with `autoload' forms.

There are four other occurrences of missing values in `setq's throughout
the source (plus another one in .../obsolete).  They all look like
deliberate "coding style", though.  If it were up to me, I would put an
explicit nil in each of these four (or five) occurrences.

I have changed the warning's wording to be more "human": it now says:

    missing value for `foo' at end of setq

.  But I do say that this warning facility I have in my personal copy
should now be committed.  It is useful.

-- 
Alan Mackenzie (Nuremberg, Germany).

RE: setq's with missing final arguments.

[Drew Adams]

> But I do say that this warning facility I have in my personal copy
> should now be committed.  It is useful.

Emacs can warn about this during byte compilation.

But it is anyway important for Emacs to raise a runtime
error if setq is passed an odd number of arguments.

Re: setq's with missing final arguments.

[Johan Bockgård]

Alan Mackenzie <acm@muc.de> writes:

> In the setq, there is a missing argument after "bar".  At the moment,
> the byte compiler just generates code to assign nil to bar, without
> giving any warning.  IMAO, this is Very Bad.

This is bug#20241.

RE: setq's with missing final arguments.

[Drew Adams]

> > In the setq, there is a missing argument after "bar".  At the moment,
> > the byte compiler just generates code to assign nil to bar, without
> > giving any warning.  IMAO, this is Very Bad.
> 
> This is bug#20241.

I'd forgotten that I filed that bug. ;-)

I disagree with the only reply to that bug report, however:

 From: Stefan Monnier
 > Beyond this particular occurrence, I think that a wrong-number-of-args
 > error should be raised when `setq' is called with an odd number of
 > arguments.

 At the very least the byte-compiler should warn about it, indeed.

A warning is _no_ substitute for raising an error.  Adding a
warning for byte compilation is worse than doing nothing, IMO.

The doc clearly says that an even number of args are _required_.

RE: setq's with missing final arguments.

[Drew Adams]

> (defun bad-setq ()
>   "Doc string"
>   (setq foo 5
> 	bar))
> In the setq, there is a missing argument after "bar".  At the moment,
> the byte compiler just generates code to assign nil to bar, without
> giving any warning.  IMAO, this is Very Bad.
> I propose to insert code into the byte compiler to detect and warn of
> this scenario:

It's not just the byte-compiler.  This is the (erroneous, IMO)
behavior of `setq', whether interpreted or byte-compiled:

(setq bar) ; bar := nil

(setq bar) should raise an error.  The doc says that `setq'
requires an even number of args.  Same thing in other Lisps
(e.g., Common Lisp).

Re: setq's with missing final arguments.

[Alan Mackenzie]

Hello, Drew.

On Sun, Nov 22, 2015 at 07:52:48AM -0800, Drew Adams wrote:
> > (defun bad-setq ()
> >   "Doc string"
> >   (setq foo 5
> > 	bar))
> > In the setq, there is a missing argument after "bar".  At the moment,
> > the byte compiler just generates code to assign nil to bar, without
> > giving any warning.  IMAO, this is Very Bad.
> > I propose to insert code into the byte compiler to detect and warn of
> > this scenario:

> It's not just the byte-compiler.  This is the (erroneous, IMO)
> behavior of `setq', whether interpreted or byte-compiled:

> (setq bar) ; bar := nil

> (setq bar) should raise an error.  The doc says that `setq'
> requires an even number of args.  Same thing in other Lisps
> (e.g., Common Lisp).

Just before midnight, I think I would agree with you.  With the help of
the above warning facility, I have located all (six) occurrences of odd
numbers of arguments to a `setq' in our sources.  One was a quite nasty
error, one is in .../lisp/obsolete, the other four are "coding style"
thingies.  It would take at least five minutes to fix them all.

I now think we should make any setq with a missing value throw an error,
whether in interpreted code or in the byte compiler.  And I think we
should do it for Emacs 25.1.

John?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: setq's with missing final arguments.

[John Wiegley]

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

> I now think we should make any setq with a missing value throw an error,
> whether in interpreted code or in the byte compiler. And I think we should
> do it for Emacs 25.1.

> John?

I agree it should cause an error, and every usage in core should be fixed.

Also please fix `setf', which has the same problem.

`set', on the other hand, gives this error:

    Error: wrong-number-of-arguments; Data: (set 1)

We should yield the same in the other two cases. And think about what other
functions should receive this same treatment.

John

RE: setq's with missing final arguments.

[Drew Adams]

> I agree it should cause an error

Great.  And then we can close bug #20241. ;-)