bug#25166: 26.0.50; It shouldn't be possible to set the function cell of nil and t

[Philipp Stephani <p.stephani2@gmail.com>]

[-- 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