Subject: Re: bug#38872: 27.0.50; Keywords can be let-bound

From: Stefan Kangas <[email-masked]>
Date: Thu, 16 Jan 2020 23:38:57 +0100

Andreas Schwab <[email-masked]> writes:

>> diff --git a/src/eval.c b/src/eval.c
>> index 4559a0e1f6..9a3f703f40 100644
>> --- a/src/eval.c
>> +++ b/src/eval.c
>> @@ -972,8 +972,11 @@ DEFUN ("let", Flet, Slet, 1, UNEVALLED, 0,
>>        if (!NILP (lexenv) && SYMBOLP (var)
>>  	  && !XSYMBOL (var)->u.s.declared_special
>>  	  && NILP (Fmemq (var, Vinternal_interpreter_environment)))
>> -	/* Lexically bind VAR by adding it to the lexenv alist.  */
>> -	lexenv = Fcons (Fcons (var, tem), lexenv);
>> +	if (XSYMBOL (var)->u.s.trapped_write == SYMBOL_NOWRITE)
>
> I think that should use the same condition as set_internal, so that (let
> ((:k :k))) still works.

Thanks, I forgot about that case.

I changed the code to just defer to the dynamic version for keywords
and added a test.  Seems to be working, and is even simpler than what
I had before.

Best regards,
Stefan Kangas

>From ea8725007633104014039f55d16737355897b5da Mon Sep 17 00:00:00 2001
From: Stefan Kangas <[email-masked]>
Date: Thu, 16 Jan 2020 15:33:08 +0100
Subject: [PATCH] Don't let-bind keywords when lexical-binding is t

* src/eval.c (Flet): Don't allow let-binding a keyword symbol when
lexical-binding is t.  This is consistent with the manual section
"(elisp)Constant variables".  (Bug#38872)

* test/src/eval-tests.el (eval-tests-let): Add tests for the let-form.
---
 src/eval.c             |  1 +
 test/src/eval-tests.el | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/src/eval.c b/src/eval.c
index 4559a0e1f6..072d01e2ea 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -971,6 +971,7 @@ DEFUN ("let", Flet, Slet, 1, UNEVALLED, 0,
 
       if (!NILP (lexenv) && SYMBOLP (var)
 	  && !XSYMBOL (var)->u.s.declared_special
+	  && (!XSYMBOL (var)->u.s.trapped_write) == SYMBOL_NOWRITE
 	  && NILP (Fmemq (var, Vinternal_interpreter_environment)))
 	/* Lexically bind VAR by adding it to the lexenv alist.  */
 	lexenv = Fcons (Fcons (var, tem), lexenv);
diff --git a/test/src/eval-tests.el b/test/src/eval-tests.el
index 074f5be1ef..f56859ec54 100644
--- a/test/src/eval-tests.el
+++ b/test/src/eval-tests.el
@@ -28,6 +28,20 @@
 (require 'ert)
 (eval-when-compile (require 'cl-lib))
 
+(ert-deftest eval-tests-let ()
+  "Test let binding using dynamic and lexical scope."
+  (dolist (nil-or-t '(nil t))
+    (message "lexical-binding: %s" nil-or-t)
+    (with-temp-buffer
+      (setq lexical-binding nil-or-t)
+      (should (equal (let ((x 1)) x) 1))
+      (should-error (let ((1 2)) x) :type '(wrong-type-argument))
+      ;; Behave consistently with (info "(elisp)Constant variables")
+      (should (equal (let ((:a :a)) :a) :a))
+      (should-error (let ((t 1)) t) :type '(setting-constant))
+      (should-error (let ((nil 1)) nil) :type '(setting-constant))
+      (should-error (let ((:a 1)) :a) :type '(setting-constant)))))
+
 (ert-deftest eval-tests--bug24673 ()
   "Check that Bug#24673 has been fixed."
   ;; This should not crash.
-- 
2.20.1