bug#59213: Emacs 29: Edebug fails to instrument a parameter whose name begins with _

[Stefan Monnier]

Hi Alan,

>  (defun cconv-make-interpreted-closure (fun env)
> +  "Make a closure for the interpreter.
> +This function is evaluated both at compile time and run time.
> +FUN, the closure's function, must be a lambda form.
> +ENV, the closure's environment, is a mixture of lexical bindings of the form
> +(SYMBOL . VALUE) and symbols which indicate dynamic bindings of those
> +symbols."

BTW, what did you mean by "This function is evaluated both at compile
time and run time"?

Also, I append the current state of the patch I plan to install on
`master`.


        Stefan


diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el
index b8121aeba55..d055026cb1a 100644
--- a/lisp/emacs-lisp/cconv.el
+++ b/lisp/emacs-lisp/cconv.el
@@ -113,10 +113,6 @@ cconv--interactive-form-funs
 (defvar cconv--dynbound-variables nil
   "List of variables known to be dynamically bound.")
 
-(defvar cconv-dont-trim-unused-variables nil
-  "When bound to non-nil, don't remove unused variables from the environment.
-This is intended for use by edebug and similar.")
-
 ;;;###autoload
 (defun cconv-closure-convert (form &optional dynbound-vars)
   "Main entry point for closure conversion.
@@ -886,11 +882,16 @@ cconv-make-interpreted-closure
 This function is evaluated both at compile time and run time.
 FUN, the closure's function, must be a lambda form.
 ENV, the closure's environment, is a mixture of lexical bindings of the form
-(SYMBOL . VALUE) and symbols which indicate dynamic bindings of those
+\(SYMBOL . VALUE) and symbols which indicate dynamic bindings of those
 symbols."
   (cl-assert (eq (car-safe fun) 'lambda))
   (let ((lexvars (delq nil (mapcar #'car-safe env))))
-    (if (or cconv-dont-trim-unused-variables (null lexvars))
+    (if (or (null lexvars)
+            ;; Functions of the form (lambda (..) :closure-dont-trim-context 
..)
+            ;; should keep their whole context untrimmed (bug#59213).
+            (and (eq :closure-dont-trim-context (nth 2 fun))
+                 ;; Check the function doesn't just return the magic keyword.
+                 (nthcdr 3 fun)))
         ;; The lexical environment is empty, or needs to be preserved,
         ;; so there's no need to look for free variables.
         ;; Attempting to replace ,(cdr fun) by a macroexpanded version
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 735a358cdba..4b625dd076e 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -1217,16 +1217,17 @@ edebug-make-enter-wrapper
     (setq edebug-old-def-name nil))
   (setq edebug-def-name
        (or edebug-def-name edebug-old-def-name (gensym "edebug-anon")))
-  `(let ((cconv-dont-trim-unused-variables t))
-     (edebug-enter
-      (quote ,edebug-def-name)
-      ,(if edebug-inside-func
-          `(list
-            ;; Doesn't work with more than one def-body!!
-            ;; But the list will just be reversed.
-            ,@(nreverse edebug-def-args))
-         'nil)
-      (function (lambda () ,@forms)))))
+  `(edebug-enter
+    (quote ,edebug-def-name)
+    ,(if edebug-inside-func
+        `(list
+          ;; Doesn't work with more than one def-body!!
+          ;; But the list will just be reversed.
+          ,@(nreverse edebug-def-args))
+       'nil)
+    ;; Make sure `forms' is not nil so we don't accidentally return
+    ;; the magic keyword.
+    #'(lambda () :closure-dont-trim-context ,@(or forms '(nil)))))
 
 
 (defvar edebug-form-begin-marker) ; the mark for def being instrumented
diff --git a/lisp/emacs-lisp/testcover.el b/lisp/emacs-lisp/testcover.el
index 1212905f08a..ed31b90ca32 100644
--- a/lisp/emacs-lisp/testcover.el
+++ b/lisp/emacs-lisp/testcover.el
@@ -442,11 +442,6 @@ testcover-analyze-coverage
      (let ((testcover-vector (get sym 'edebug-coverage)))
        (testcover-analyze-coverage-progn body)))
 
-    (`(let ((cconv-dont-trim-unused-variables t))
-        (edebug-enter ',sym ,_ (function (lambda nil . ,body))))
-     (let ((testcover-vector (get sym 'edebug-coverage)))
-       (testcover-analyze-coverage-progn body)))
-
     (`(edebug-after ,(and before-form
                           (or `(edebug-before ,before-id) before-id))
                     ,after-id ,wrapped-form)
diff --git a/test/lisp/emacs-lisp/cconv-tests.el 
b/test/lisp/emacs-lisp/cconv-tests.el
index 83013cf46a9..349ffeb7e47 100644
--- a/test/lisp/emacs-lisp/cconv-tests.el
+++ b/test/lisp/emacs-lisp/cconv-tests.el
@@ -364,5 +364,18 @@ cconv-tests--intern-all
                            (call-interactively f))
                      '((t 51696) (nil 51695) (t 51697)))))))
 
+(ert-deftest cconv-safe-for-space ()
+  (let* ((magic-string "This-is-a-magic-string")
+         (safe-p (lambda (x) (not (string-match magic-string (format "%S" 
x))))))
+    (should (funcall safe-p (lambda (x) (+ x 1))))
+    (should (funcall safe-p (eval '(lambda (x) (+ x 1))
+                                  `((y . ,magic-string)))))
+    (should (funcall safe-p (eval '(lambda (x) :closure-dont-trim-context)
+                                  `((y . ,magic-string)))))
+    (should-not (funcall safe-p
+                         (eval '(lambda (x) :closure-dont-trim-context (+ x 1))
+                               `((y . ,magic-string)))))))
+
+
 (provide 'cconv-tests)
 ;;; cconv-tests.el ends here