* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
@ 2016-12-11 9:41 Philipp
2016-12-11 18:38 ` Glenn Morris
0 siblings, 1 reply; 15+ messages in thread
From: Philipp @ 2016-12-11 9:41 UTC (permalink / raw)
To: 25166
You can set the function cell of nil and t using `fset' and friends.
But you can't call the `nil' function using (nil) (it does work with
(t)). I think that attempting to set the function cell of nil and t is
almost always a bug -- probably the programmer wanted to set a real
symbol, but some of the constants got passed. I propose to signal an
error (e.g. `setting-constant') whenever the function cell of nil and t
is modified; maybe the same should happen for keywords.
In GNU Emacs 26.0.50.5 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
of 2016-12-10 built on localhost
Repository revision: dc62ea38e36b5de47893a60a76d66b7a79bf6437
Windowing system distributor 'The X.Org Foundation', version 11.0.11702000
System Description: Ubuntu 14.04 LTS
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Configured using:
'configure --enable-checking --enable-check-lisp-object-type
'CFLAGS=-O0 -ggdb3''
Configured features:
XPM JPEG TIFF GIF PNG SOUND GSETTINGS NOTIFY GNUTLS FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11
Important settings:
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8-unix
Major mode: Lisp Interaction
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
line-number-mode: t
transient-mark-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message subr-x puny seq byte-opt gv
bytecomp byte-compile cl-extra help-mode cconv cl-loaddefs pcase cl-lib
dired dired-loaddefs format-spec rfc822 mml easymenu mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)
Memory information:
((conses 16 97085 5586)
(symbols 48 20041 0)
(miscs 40 331 105)
(strings 32 17859 3593)
(string-bytes 1 580498)
(vectors 16 13985)
(vector-slots 8 448755 7198)
(floats 8 179 25)
(intervals 56 201 0)
(buffers 976 11)
(heap 1024 47360 1053))
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-11 9:41 bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t Philipp
@ 2016-12-11 18:38 ` Glenn Morris
2016-12-12 20:25 ` Glenn Morris
0 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2016-12-11 18:38 UTC (permalink / raw)
To: Philipp; +Cc: 25166
Philipp wrote:
> You can set the function cell of nil and t using `fset' and friends.
> But you can't call the `nil' function using (nil) (it does work with
> (t)). I think that attempting to set the function cell of nil and t is
> almost always a bug -- probably the programmer wanted to set a real
> symbol, but some of the constants got passed. I propose to signal an
> error (e.g. `setting-constant') whenever the function cell of nil and t
> is modified; maybe the same should happen for keywords.
I just did this yesterday...
See https://debbugs.gnu.org/25110, ba8e883, and 3fd4433.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-11 18:38 ` Glenn Morris
@ 2016-12-12 20:25 ` Glenn Morris
2016-12-26 19:29 ` Philipp Stephani
0 siblings, 1 reply; 15+ messages in thread
From: Glenn Morris @ 2016-12-12 20:25 UTC (permalink / raw)
To: Philipp; +Cc: 25166
Glenn Morris wrote:
> Philipp wrote:
>
>> You can set the function cell of nil and t using `fset' and friends.
>> But you can't call the `nil' function using (nil) (it does work with
>> (t)). I think that attempting to set the function cell of nil and t is
>> almost always a bug -- probably the programmer wanted to set a real
>> symbol, but some of the constants got passed. I propose to signal an
>> error (e.g. `setting-constant') whenever the function cell of nil and t
>> is modified; maybe the same should happen for keywords.
>
> I just did this yesterday...
> See https://debbugs.gnu.org/25110, ba8e883, and 3fd4433.
And now see also ffb1302. :)
Anyway, I only did "nil", since as you say "t" can actually be called as
a function. But you are right that it's probably unintended.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-12 20:25 ` Glenn Morris
@ 2016-12-26 19:29 ` Philipp Stephani
2016-12-26 19:41 ` Philipp Stephani
2016-12-26 19:47 ` Eli Zaretskii
0 siblings, 2 replies; 15+ messages in thread
From: Philipp Stephani @ 2016-12-26 19:29 UTC (permalink / raw)
To: Glenn Morris; +Cc: 25166
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
Glenn Morris <rgm@gnu.org> schrieb am Mo., 12. Dez. 2016 um 21:25 Uhr:
> Glenn Morris wrote:
>
> > Philipp wrote:
> >
> >> You can set the function cell of nil and t using `fset' and friends.
> >> But you can't call the `nil' function using (nil) (it does work with
> >> (t)). I think that attempting to set the function cell of nil and t is
> >> almost always a bug -- probably the programmer wanted to set a real
> >> symbol, but some of the constants got passed. I propose to signal an
> >> error (e.g. `setting-constant') whenever the function cell of nil and t
> >> is modified; maybe the same should happen for keywords.
> >
> > I just did this yesterday...
> > See https://debbugs.gnu.org/25110, ba8e883, and 3fd4433.
>
> And now see also ffb1302. :)
> Anyway, I only did "nil", since as you say "t" can actually be called as
> a function. But you are right that it's probably unintended.
>
>
Thanks! Interestingly the behavior was already added in 1994 (commit
c15c5d408d696928862ca2848a359231e373556c), but apparently reverted later.
I'd suggest to simply reinstate that commit.
[-- Attachment #2: Type: text/html, Size: 1966 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-26 19:29 ` Philipp Stephani
@ 2016-12-26 19:41 ` Philipp Stephani
2016-12-26 19:47 ` Eli Zaretskii
1 sibling, 0 replies; 15+ messages in thread
From: Philipp Stephani @ 2016-12-26 19:41 UTC (permalink / raw)
To: Glenn Morris; +Cc: 25166
[-- Attachment #1.1: Type: text/plain, Size: 1205 bytes --]
Philipp Stephani <p.stephani2@gmail.com> schrieb am Mo., 26. Dez. 2016 um
20:29 Uhr:
> Glenn Morris <rgm@gnu.org> schrieb am Mo., 12. Dez. 2016 um 21:25 Uhr:
>
> Glenn Morris wrote:
>
> > Philipp wrote:
> >
> >> You can set the function cell of nil and t using `fset' and friends.
> >> But you can't call the `nil' function using (nil) (it does work with
> >> (t)). I think that attempting to set the function cell of nil and t is
> >> almost always a bug -- probably the programmer wanted to set a real
> >> symbol, but some of the constants got passed. I propose to signal an
> >> error (e.g. `setting-constant') whenever the function cell of nil and t
> >> is modified; maybe the same should happen for keywords.
> >
> > I just did this yesterday...
> > See https://debbugs.gnu.org/25110, ba8e883, and 3fd4433.
>
> And now see also ffb1302. :)
> Anyway, I only did "nil", since as you say "t" can actually be called as
> a function. But you are right that it's probably unintended.
>
>
> Thanks! Interestingly the behavior was already added in 1994 (commit
> c15c5d408d696928862ca2848a359231e373556c), but apparently reverted later.
> I'd suggest to simply reinstate that commit.
>
Here's a patch.
[-- Attachment #1.2: Type: text/html, Size: 2569 bytes --]
[-- Attachment #2: 0001-Prevent-setting-the-function-cell-of-t-Bug-25166.txt --]
[-- Type: text/plain, Size: 1713 bytes --]
From 5e47f4f2af44285fc877906c5c7689f66e80baf6 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Mon, 26 Dec 2016 20:38:04 +0100
Subject: [PATCH] Prevent setting the function cell of t (Bug#25166)
Restore behavior of commit c15c5d408d696928862ca2848a359231e373556c,
and make 'fset' consistent with 'fmakunbound'.
* src/data.c (Ffset): Also disallow setting the function cell of t.
* test/src/data-tests.el (data-tests-fset-fmakunbound): Add unit test.
---
src/data.c | 2 +-
test/src/data-tests.el | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/data.c b/src/data.c
index 821fc37937..0fe4bdf590 100644
--- a/src/data.c
+++ b/src/data.c
@@ -734,7 +734,7 @@ DEFUN ("fset", Ffset, Sfset, 2, 2, 0,
register Lisp_Object function;
CHECK_SYMBOL (symbol);
/* Perhaps not quite the right error signal, but seems good enough. */
- if (NILP (symbol))
+ if (NILP (symbol) || EQ (symbol, Qt))
xsignal1 (Qsetting_constant, symbol);
function = XSYMBOL (symbol)->function;
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index 757522e399..2daba91b8f 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -450,3 +450,10 @@ binding-test-some-local
(remove-variable-watcher 'data-tests-lvar collect-watch-data)
(setq data-tests-lvar 6)
(should (null watch-data)))))
+
+(ert-deftest data-tests-fset-fmakunbound ()
+ "Test that Bug#25166 is fixed."
+ (should-error (fset nil #'car) :type 'setting-constant)
+ (should-error (fset t #'car) :type 'setting-constant)
+ (should-error (fmakunbound nil) :type 'setting-constant)
+ (should-error (fmakunbound t) :type 'setting-constant))
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-26 19:29 ` Philipp Stephani
2016-12-26 19:41 ` Philipp Stephani
@ 2016-12-26 19:47 ` Eli Zaretskii
2016-12-27 15:15 ` Stefan Monnier
1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-12-26 19:47 UTC (permalink / raw)
To: Philipp Stephani, Stefan Monnier; +Cc: 25166
> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 26 Dec 2016 19:29:17 +0000
> Cc: 25166@debbugs.gnu.org
>
> And now see also ffb1302. :)
> Anyway, I only did "nil", since as you say "t" can actually be called as
> a function. But you are right that it's probably unintended.
>
> Thanks! Interestingly the behavior was already added in 1994 (commit
> c15c5d408d696928862ca2848a359231e373556c), but apparently reverted later. I'd suggest to simply
> reinstate that commit.
It was reverted in 32e5c58ca969ec30d31520da52c9866cafa62927. Perhaps
Stefan could tell why he did that (the log message doesn't say).
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-26 19:47 ` Eli Zaretskii
@ 2016-12-27 15:15 ` Stefan Monnier
2016-12-27 22:49 ` Eli Zaretskii
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2016-12-27 15:15 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Philipp Stephani, 25166
> It was reverted in 32e5c58ca969ec30d31520da52c9866cafa62927. Perhaps
> Stefan could tell why he did that (the log message doesn't say).
Because there doesn't seem to be any good reason to single out nil and
t in this respect and because those things aren't terribly dangerous.
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-27 15:15 ` Stefan Monnier
@ 2016-12-27 22:49 ` Eli Zaretskii
2016-12-27 23:11 ` Stefan Monnier
0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-12-27 22:49 UTC (permalink / raw)
To: Stefan Monnier; +Cc: p.stephani2, 25166
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: Philipp Stephani <p.stephani2@gmail.com>, rgm@gnu.org,
> 25166@debbugs.gnu.org
> Date: Tue, 27 Dec 2016 10:15:29 -0500
>
> > It was reverted in 32e5c58ca969ec30d31520da52c9866cafa62927. Perhaps
> > Stefan could tell why he did that (the log message doesn't say).
>
> Because there doesn't seem to be any good reason to single out nil and
> t in this respect and because those things aren't terribly dangerous.
So therefore we shouldn't reinstate that now.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-27 22:49 ` Eli Zaretskii
@ 2016-12-27 23:11 ` Stefan Monnier
2017-05-01 14:44 ` Philipp Stephani
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2016-12-27 23:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: p.stephani2, 25166
>> Because there doesn't seem to be any good reason to single out nil and
>> t in this respect and because those things aren't terribly dangerous.
> So therefore we shouldn't reinstate that now.
Yes, that's my opinion.
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2016-12-27 23:11 ` Stefan Monnier
@ 2017-05-01 14:44 ` Philipp Stephani
2017-05-01 15:12 ` Eli Zaretskii
0 siblings, 1 reply; 15+ messages in thread
From: Philipp Stephani @ 2017-05-01 14:44 UTC (permalink / raw)
To: Stefan Monnier, Eli Zaretskii; +Cc: 25166
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 28. Dez. 2016 um
00:11 Uhr:
> >> Because there doesn't seem to be any good reason to single out nil and
> >> t in this respect and because those things aren't terribly dangerous.
> > So therefore we shouldn't reinstate that now.
>
> Yes, that's my opinion.
>
>
>
Well, my opinion is obviously different, so the maintainer should decide.
[-- Attachment #2: Type: text/html, Size: 727 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2017-05-01 14:44 ` Philipp Stephani
@ 2017-05-01 15:12 ` Eli Zaretskii
2017-10-27 17:59 ` Noam Postavsky
0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2017-05-01 15:12 UTC (permalink / raw)
To: Philipp Stephani; +Cc: 25166, monnier
> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 01 May 2017 14:44:44 +0000
> Cc: rgm@gnu.org, 25166@debbugs.gnu.org
>
> Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 28. Dez. 2016 um 00:11 Uhr:
>
> >> Because there doesn't seem to be any good reason to single out nil and
> >> t in this respect and because those things aren't terribly dangerous.
> > So therefore we shouldn't reinstate that now.
>
> Yes, that's my opinion.
>
> Well, my opinion is obviously different, so the maintainer should decide.
One maintainer has already decided, see above.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2017-05-01 15:12 ` Eli Zaretskii
@ 2017-10-27 17:59 ` Noam Postavsky
2017-10-28 16:16 ` Stefan Monnier
0 siblings, 1 reply; 15+ messages in thread
From: Noam Postavsky @ 2017-10-27 17:59 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Philipp Stephani, 25166, Stefan Monnier
# fset of nil is already covered in Bug#25110
retitle 25166 It shouldn't be possible to set the function cell of t
tags 25166 wontfix
close 25166
quit
On Mon, May 1, 2017 at 11:12 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Philipp Stephani <p.stephani2@gmail.com>
>> Date: Mon, 01 May 2017 14:44:44 +0000
>> Cc: rgm@gnu.org, 25166@debbugs.gnu.org
>>
>> Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Mi., 28. Dez. 2016 um 00:11 Uhr:
>>
>> >> Because there doesn't seem to be any good reason to single out nil and
>> >> t in this respect and because those things aren't terribly dangerous.
>> > So therefore we shouldn't reinstate that now.
>>
>> Yes, that's my opinion.
>>
>> Well, my opinion is obviously different, so the maintainer should decide.
>
> One maintainer has already decided, see above.
Seems final enough, closing as wontfix.
PS for an example of why singling out nil makes sense (but this
doesn't apply to t) see [1].
Summary: (autoload #'nil "something" nil t) silently succeeds but
makes forward-word, backward-word, etc, (fail to) funcall nil.
[1]: https://github.com/jwiegley/use-package/issues/512
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2017-10-27 17:59 ` Noam Postavsky
@ 2017-10-28 16:16 ` Stefan Monnier
2017-10-28 16:45 ` Noam Postavsky
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Monnier @ 2017-10-28 16:16 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 25166, Philipp Stephani
> PS for an example of why singling out nil makes sense (but this
> doesn't apply to t) see [1].
> Summary: (autoload #'nil "something" nil t) silently succeeds but
> makes forward-word, backward-word, etc, (fail to) funcall nil.
Why would someone do (autoload #'nil "something" nil t) ?
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2017-10-28 16:16 ` Stefan Monnier
@ 2017-10-28 16:45 ` Noam Postavsky
2017-10-29 18:10 ` Stefan Monnier
0 siblings, 1 reply; 15+ messages in thread
From: Noam Postavsky @ 2017-10-28 16:45 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 25166, Philipp Stephani
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> PS for an example of why singling out nil makes sense (but this
>> doesn't apply to t) see [1].
>> Summary: (autoload #'nil "something" nil t) silently succeeds but
>> makes forward-word, backward-word, etc, (fail to) funcall nil.
>
> Why would someone do (autoload #'nil "something" nil t) ?
By accident. It's a bug in use-package.
(use-package foo-pkg :commands <commands-to-autoload>)
Creates autoloads for all the commands in <commands-to-autoload>, which
can be a symbol, or a list of symbols. But nil is also a symbol, so
passing an empty list makes an autoload for nil (because use-package
checks for symbolp before listp), rather than making autoloads for 0
commands.
^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t
2017-10-28 16:45 ` Noam Postavsky
@ 2017-10-29 18:10 ` Stefan Monnier
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Monnier @ 2017-10-29 18:10 UTC (permalink / raw)
To: Noam Postavsky; +Cc: 25166, Philipp Stephani
> By accident. It's a bug in use-package.
That's a rare accident, which I assume has been fixed already (if not,
it's another argument in favor of saying that it's not important).
Nothing that warrants adding special error detection.
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-10-29 18:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-11 9:41 bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t Philipp
2016-12-11 18:38 ` Glenn Morris
2016-12-12 20:25 ` Glenn Morris
2016-12-26 19:29 ` Philipp Stephani
2016-12-26 19:41 ` Philipp Stephani
2016-12-26 19:47 ` Eli Zaretskii
2016-12-27 15:15 ` Stefan Monnier
2016-12-27 22:49 ` Eli Zaretskii
2016-12-27 23:11 ` Stefan Monnier
2017-05-01 14:44 ` Philipp Stephani
2017-05-01 15:12 ` Eli Zaretskii
2017-10-27 17:59 ` Noam Postavsky
2017-10-28 16:16 ` Stefan Monnier
2017-10-28 16:45 ` Noam Postavsky
2017-10-29 18:10 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).