* 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).