[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#59213: Emacs 29: Edebug fails to instrument a parameter whose name b
From: |
Stefan Monnier |
Subject: |
bug#59213: Emacs 29: Edebug fails to instrument a parameter whose name begins with _ |
Date: |
Tue, 14 Feb 2023 17:19:12 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) |
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