emacs-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

master a350ae0: * lisp/emacs-lisp/cconv.el: Improve line-nb info of unus


From: Stefan Monnier
Subject: master a350ae0: * lisp/emacs-lisp/cconv.el: Improve line-nb info of unused var warnings
Date: Fri, 26 Feb 2021 20:24:58 -0500 (EST)

branch: master
commit a350ae058caedcb7be7d332564817954e3624e60
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    * lisp/emacs-lisp/cconv.el: Improve line-nb info of unused var warnings
    
    Instead of warning about unused vars during the analysis phase of
    closure conversion, do it in the actual closure conversion by
    annotating the code with "unused" warnings, so that the warnings
    get emitted later by the bytecomp phase, like all other warnings,
    at which point the line-number info is a bit less imprecise.
    
    Take advantage of this change to wrap the expressions of unused
    let-bound vars inside (ignore ...) so the byte-compiler can better
    optimize them away.
    
    Finally, promote `macroexp--warn-and-return` to "official" status
    by removing its "--" marker.
    
    (cconv-captured+mutated, cconv-lambda-candidates): Remove vars.
    (cconv-var-classification): New var to replace them.
    (cconv-warnings-only): Delete function.
    (cconv--warn-unused-msg, cconv--var-classification): New functions.
    (cconv--convert-funcbody): Add warnings for unused args.
    (cconv-convert): Add warnings for unused vars in `let` and `condition-case`.
    (cconv--analyze-use): Don't emit an "unused var" warning any more,
    but instead remember the fact in `cconv-var-classification`.
    
    * lisp/emacs-lisp/bytecomp.el (byte-compile-force-lexical-warnings):
    Remove variable.
    (byte-compile-preprocess): Remove corresponding case.
    
    * lisp/emacs-lisp/pcase.el (pcase--if): Don't throw away `test` effects.
    (\`):
    * lisp/emacs-lisp/cl-macs.el (cl--do-arglist): Use `car-safe` instead
    of `car`, so it can more easily be removed by the optimizer if the
    result is not used.
    
    * lisp/emacs-lisp/macroexp.el (macroexp--warn-wrap): New function.
    (macroexp-warn-and-return): Rename from `macroexp--warn-and-return`.
---
 etc/NEWS                      |   4 +
 lisp/emacs-lisp/byte-run.el   |   4 +-
 lisp/emacs-lisp/bytecomp.el   |   3 -
 lisp/emacs-lisp/cconv.el      | 211 ++++++++++++++++++++++++------------------
 lisp/emacs-lisp/cl-generic.el |   2 +-
 lisp/emacs-lisp/cl-macs.el    |   8 +-
 lisp/emacs-lisp/eieio-core.el |   2 +-
 lisp/emacs-lisp/eieio.el      |   2 +-
 lisp/emacs-lisp/gv.el         |   2 +-
 lisp/emacs-lisp/inline.el     |   2 +-
 lisp/emacs-lisp/macroexp.el   |  57 ++++++------
 lisp/emacs-lisp/pcase.el      |  12 ++-
 lisp/progmodes/elisp-mode.el  |   1 +
 13 files changed, 173 insertions(+), 137 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f8f41e2..cb30767 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -374,6 +374,10 @@ the buffer cycles the whole buffer between "only top-level 
headings",
 *** New function 'macroexp-file-name' to know the name of the current file
 ---
 *** New function 'macroexp-compiling-p' to know if we're compiling.
+---
+*** New function 'macroexp-warn-and-return' to help emit warnings.
+This used to be named 'macroexp--warn-and-return' and has proved useful
+and well-behaved enough to lose the "internal" marker.
 
 ** 'blink-cursor-mode' is now enabled by default regardless of the UI.
 It used to be enabled when Emacs is started in GUI mode but not when started
diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index 6451d7f..119d397 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -247,7 +247,7 @@ The return value is undefined.
                  #'(lambda (x)
                      (let ((f (cdr (assq (car x) macro-declarations-alist))))
                        (if f (apply (car f) name arglist (cdr x))
-                         (macroexp--warn-and-return
+                         (macroexp-warn-and-return
                           (format-message
                            "Unknown macro property %S in %S"
                            (car x) name)
@@ -320,7 +320,7 @@ The return value is undefined.
                               body)))
                     nil)
                    (t
-                    (macroexp--warn-and-return
+                    (macroexp-warn-and-return
                      (format-message "Unknown defun property `%S' in %S"
                                      (car x) name)
                      nil)))))
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 7aae8c0..f859795 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -2422,8 +2422,6 @@ list that represents a doc string reference.
              byte-compile-output nil
               byte-compile-jump-tables nil))))
 
-(defvar byte-compile-force-lexical-warnings nil)
-
 (defun byte-compile-preprocess (form &optional _for-effect)
   (setq form (macroexpand-all form byte-compile-macro-environment))
   ;; FIXME: We should run byte-optimize-form here, but it currently does not
@@ -2434,7 +2432,6 @@ list that represents a doc string reference.
   ;;     (setq form (byte-optimize-form form for-effect)))
   (cond
    (lexical-binding (cconv-closure-convert form))
-   (byte-compile-force-lexical-warnings (cconv-warnings-only form))
    (t form)))
 
 ;; byte-hunk-handlers cannot call this!
diff --git a/lisp/emacs-lisp/cconv.el b/lisp/emacs-lisp/cconv.el
index e795839..7b525b7 100644
--- a/lisp/emacs-lisp/cconv.el
+++ b/lisp/emacs-lisp/cconv.el
@@ -121,19 +121,22 @@
 (defconst cconv-liftwhen 6
   "Try to do lambda lifting if the number of arguments + free variables
 is less than this number.")
-;; List of all the variables that are both captured by a closure
-;; and mutated.  Each entry in the list takes the form
-;; (BINDER . PARENTFORM) where BINDER is the (VAR VAL) that introduces the
-;; variable (or is just (VAR) for variables not introduced by let).
-(defvar cconv-captured+mutated)
-
-;; List of candidates for lambda lifting.
-;; Each candidate has the form (BINDER . PARENTFORM).  A candidate
-;; is a variable that is only passed to `funcall' or `apply'.
-(defvar cconv-lambda-candidates)
-
-;; Alist associating to each function body the list of its free variables.
-(defvar cconv-freevars-alist)
+(defvar cconv-var-classification
+  ;; Alist mapping variables to a given class.
+  ;; The keys are of the form (BINDER . PARENTFORM) where BINDER
+  ;; is the (VAR VAL) that introduces it (or is just (VAR) for variables
+  ;; not introduced by let).
+  ;; The class can be one of:
+  ;; - :unused
+  ;; - :lambda-candidate
+  ;; - :captured+mutated
+  ;; - nil for "normal" variables, which would then just not appear
+  ;;   in the alist at all.
+  )
+
+(defvar cconv-freevars-alist
+  ;; Alist associating to each function body the list of its free variables.
+  )
 
 ;;;###autoload
 (defun cconv-closure-convert (form)
@@ -144,25 +147,13 @@ is less than this number.")
 Returns a form where all lambdas don't have any free variables."
   ;; (message "Entering cconv-closure-convert...")
   (let ((cconv-freevars-alist '())
-       (cconv-lambda-candidates '())
-       (cconv-captured+mutated '()))
+       (cconv-var-classification '()))
     ;; Analyze form - fill these variables with new information.
     (cconv-analyze-form form '())
     (setq cconv-freevars-alist (nreverse cconv-freevars-alist))
     (prog1 (cconv-convert form nil nil) ; Env initially empty.
       (cl-assert (null cconv-freevars-alist)))))
 
-;;;###autoload
-(defun cconv-warnings-only (form)
-  "Add the warnings that closure conversion would encounter."
-  (let ((cconv-freevars-alist '())
-       (cconv-lambda-candidates '())
-       (cconv-captured+mutated '()))
-    ;; Analyze form - fill these variables with new information.
-    (cconv-analyze-form form '())
-    ;; But don't perform the closure conversion.
-    form))
-
 (defconst cconv--dummy-var (make-symbol "ignored"))
 
 (defun cconv--set-diff (s1 s2)
@@ -261,28 +252,55 @@ Returns a form where all lambdas don't have any free 
variables."
                           (nthcdr 3 mapping)))))
           new-env))
 
+(defun cconv--warn-unused-msg (var varkind)
+  (unless (or ;; Uninterned symbols typically come from macro-expansion, so
+              ;; it is often non-trivial for the programmer to avoid such
+              ;; unused vars.
+              (not (intern-soft var))
+              (eq ?_ (aref (symbol-name var) 0))
+             ;; As a special exception, ignore "ignore".
+             (eq var 'ignored))
+       (let ((suggestions (help-uni-confusable-suggestions (symbol-name var))))
+         (format "Unused lexical %s `%S'%s"
+                 varkind var
+                 (if suggestions (concat "\n  " suggestions) "")))))
+
+(define-inline cconv--var-classification (binder form)
+  (inline-quote
+   (alist-get (cons ,binder ,form) cconv-var-classification
+              nil nil #'equal)))
+
 (defun cconv--convert-funcbody (funargs funcbody env parentform)
   "Run `cconv-convert' on FUNCBODY, the forms of a lambda expression.
 PARENTFORM is the form containing the lambda expression.  ENV is a
 lexical environment (same format as for `cconv-convert'), not
 including FUNARGS, the function's argument list.  Return a list
 of converted forms."
-  (let ((letbind ()))
+  (let ((wrappers ()))
     (dolist (arg funargs)
-      (if (not (member (cons (list arg) parentform) cconv-captured+mutated))
-          (if (assq arg env) (push `(,arg . nil) env))
-        (push `(,arg . (car-safe ,arg)) env)
-        (push `(,arg (list ,arg)) letbind)))
+      (pcase (cconv--var-classification (list arg) parentform)
+        (:captured+mutated
+         (push `(,arg . (car-safe ,arg)) env)
+         (push (lambda (body) `(let ((,arg (list ,arg))) ,body)) wrappers))
+        ((and :unused
+              (let (and (pred stringp) msg)
+                (cconv--warn-unused-msg arg "argument")))
+         (if (assq arg env) (push `(,arg . nil) env)) ;FIXME: Is it needed?
+         (push (lambda (body) `(macroexp--warn-wrap ,msg ,body)) wrappers))
+        (_
+         (if (assq arg env) (push `(,arg . nil) env)))))
     (setq funcbody (mapcar (lambda (form)
                              (cconv-convert form env nil))
                            funcbody))
-    (if letbind
+    (if wrappers
         (let ((special-forms '()))
           ;; Keep special forms at the beginning of the body.
           (while (or (stringp (car funcbody)) ;docstring.
                      (memq (car-safe (car funcbody)) '(interactive declare)))
             (push (pop funcbody) special-forms))
-          `(,@(nreverse special-forms) (let ,letbind . ,funcbody)))
+          (let ((body (macroexp-progn funcbody)))
+            (dolist (wrapper wrappers) (setq body (funcall wrapper body)))
+            `(,@(nreverse special-forms) ,@(macroexp-unprogn body))))
       funcbody)))
 
 (defun cconv-convert (form env extend)
@@ -340,46 +358,58 @@ places where they originally did not directly appear."
                       (setq value (cadr binder))
                       (car binder)))
                (new-val
-                (cond
-                  ;; Check if var is a candidate for lambda lifting.
-                  ((and (member (cons binder form) cconv-lambda-candidates)
-                        (progn
-                          (cl-assert (and (eq (car value) 'function)
-                                          (eq (car (cadr value)) 'lambda)))
-                          (cl-assert (equal (cddr (cadr value))
-                                            (caar cconv-freevars-alist)))
-                          ;; Peek at the freevars to decide whether to λ-lift.
-                          (let* ((fvs (cdr (car cconv-freevars-alist)))
-                                 (fun (cadr value))
-                                 (funargs (cadr fun))
-                                 (funcvars (append fvs funargs)))
+                (pcase (cconv--var-classification binder form)
+                   ;; Check if var is a candidate for lambda lifting.
+                   ((and :lambda-candidate
+                         (guard
+                          (progn
+                            (cl-assert (and (eq (car value) 'function)
+                                            (eq (car (cadr value)) 'lambda)))
+                            (cl-assert (equal (cddr (cadr value))
+                                              (caar cconv-freevars-alist)))
+                            ;; Peek at the freevars to decide whether to 
λ-lift.
+                            (let* ((fvs (cdr (car cconv-freevars-alist)))
+                                   (fun (cadr value))
+                                   (funargs (cadr fun))
+                                   (funcvars (append fvs funargs)))
                                        ; lambda lifting condition
-                            (and fvs (>= cconv-liftwhen (length funcvars))))))
+                              (and fvs (>= cconv-liftwhen
+                                          (length funcvars)))))))
                                        ; Lift.
-                   (let* ((fvs (cdr (pop cconv-freevars-alist)))
-                          (fun (cadr value))
-                          (funargs (cadr fun))
-                          (funcvars (append fvs funargs))
-                          (funcbody (cddr fun))
-                          (funcbody-env ()))
-                     (push `(,var . (apply-partially ,var . ,fvs)) new-env)
-                     (dolist (fv fvs)
-                       (cl-pushnew fv new-extend)
-                       (if (and (eq 'car-safe (car-safe (cdr (assq fv env))))
-                                (not (memq fv funargs)))
-                           (push `(,fv . (car-safe ,fv)) funcbody-env)))
-                     `(function (lambda ,funcvars .
-                                  ,(cconv--convert-funcbody
-                                    funargs funcbody funcbody-env value)))))
+                    (let* ((fvs (cdr (pop cconv-freevars-alist)))
+                           (fun (cadr value))
+                           (funargs (cadr fun))
+                           (funcvars (append fvs funargs))
+                           (funcbody (cddr fun))
+                           (funcbody-env ()))
+                      (push `(,var . (apply-partially ,var . ,fvs)) new-env)
+                      (dolist (fv fvs)
+                        (cl-pushnew fv new-extend)
+                        (if (and (eq 'car-safe (car-safe (cdr (assq fv env))))
+                                 (not (memq fv funargs)))
+                            (push `(,fv . (car-safe ,fv)) funcbody-env)))
+                      `(function (lambda ,funcvars .
+                                   ,(cconv--convert-funcbody
+                                     funargs funcbody funcbody-env value)))))
 
                   ;; Check if it needs to be turned into a "ref-cell".
-                  ((member (cons binder form) cconv-captured+mutated)
+                  (:captured+mutated
                    ;; Declared variable is mutated and captured.
                    (push `(,var . (car-safe ,var)) new-env)
                    `(list ,(cconv-convert value env extend)))
 
+                  ;; Check if it needs to be turned into a "ref-cell".
+                  (:unused
+                   ;; Declared variable is unused.
+                   (if (assq var new-env) (push `(,var) new-env)) 
;FIXME:Needed?
+                   (let ((newval
+                          `(ignore ,(cconv-convert value env extend)))
+                         (msg (cconv--warn-unused-msg var "variable")))
+                     (if (null msg) newval
+                       (macroexp--warn-wrap msg newval))))
+
                   ;; Normal default case.
-                  (t
+                  (_
                    (if (assq var new-env) (push `(,var) new-env))
                    (cconv-convert value env extend)))))
 
@@ -464,22 +494,28 @@ places where they originally did not directly appear."
 
                                         ; condition-case
     (`(condition-case ,var ,protected-form . ,handlers)
-     `(condition-case ,var
-          ,(cconv-convert protected-form env extend)
-        ,@(let* ((cm (and var (member (cons (list var) form)
-                                      cconv-captured+mutated)))
-                 (newenv
-                  (cond (cm (cons `(,var . (car-save ,var)) env))
-                        ((assq var env) (cons `(,var) env))
-                        (t env))))
-            (mapcar
+     (let* ((class (and var (cconv--var-classification (list var) form)))
+            (newenv
+             (cond ((eq class :captured+mutated)
+                    (cons `(,var . (car-save ,var)) env))
+                   ((assq var env) (cons `(,var) env))
+                   (t env)))
+            (msg (when (eq class :unused)
+                   (cconv--warn-unused-msg var "variable")))
+            (newprotform (cconv-convert protected-form env extend)))
+       `(condition-case ,var
+            ,(if msg
+                 `(macroexp--warn-wrap msg newprotform)
+               newprotform)
+          ,@(mapcar
              (lambda (handler)
                `(,(car handler)
                  ,@(let ((body
                           (mapcar (lambda (form)
                                     (cconv-convert form newenv extend))
                                   (cdr handler))))
-                     (if (not cm) body
+                     (if (not (eq class :captured+mutated))
+                         body
                        `((let ((,var (list ,var))) ,@body))))))
              handlers))))
 
@@ -563,29 +599,21 @@ FORM is the parent form that binds this var."
     (`(,_ nil nil nil nil) nil)
     (`((,(and var (guard (eq ?_ (aref (symbol-name var) 0)))) . ,_)
        ,_ ,_ ,_ ,_)
+     ;; FIXME: Convert this warning to use `macroexp--warn-wrap'
+     ;; so as to give better position information.
      (byte-compile-warn
       "%s `%S' not left unused" varkind var)))
   (pcase vardata
-    (`((,var . ,_) nil ,_ ,_ nil)
-     ;; FIXME: This gives warnings in the wrong order, with imprecise line
-     ;; numbers and without function name info.
-     (unless (or ;; Uninterned symbols typically come from macro-expansion, so
-              ;; it is often non-trivial for the programmer to avoid such
-              ;; unused vars.
-              (not (intern-soft var))
-              (eq ?_ (aref (symbol-name var) 0))
-             ;; As a special exception, ignore "ignore".
-             (eq var 'ignored))
-       (let ((suggestions (help-uni-confusable-suggestions (symbol-name var))))
-         (byte-compile-warn "Unused lexical %s `%S'%s"
-                            varkind var
-                            (if suggestions (concat "\n  " suggestions) "")))))
+    (`(,binder nil ,_ ,_ nil)
+     (push (cons (cons binder form) :unused) cconv-var-classification))
     ;; If it's unused, there's no point converting it into a cons-cell, even if
     ;; it's captured and mutated.
     (`(,binder ,_ t t ,_)
-     (push (cons binder form) cconv-captured+mutated))
+     (push (cons (cons binder form) :captured+mutated)
+           cconv-var-classification))
     (`(,(and binder `(,_ (function (lambda . ,_)))) nil nil nil t)
-     (push (cons binder form) cconv-lambda-candidates))))
+     (push (cons (cons binder form) :lambda-candidates)
+           cconv-var-classification))))
 
 (defun cconv--analyze-function (args body env parentform)
   (let* ((newvars nil)
@@ -638,8 +666,7 @@ Analyze lambdas if they are suitable for lambda lifting.
 - ENV is an alist mapping each enclosing lexical variable to its info.
    I.e. each element has the form (VAR . (READ MUTATED CAPTURED CALLED)).
 This function does not return anything but instead fills the
-`cconv-captured+mutated' and `cconv-lambda-candidates' variables
-and updates the data stored in ENV."
+`cconv-var-classification' variable and updates the data stored in ENV."
   (pcase form
                                        ; let special form
     (`(,(and (or 'let* 'let) letsym) ,binders . ,body-forms)
diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index 279b9d1..89fc0b1 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -487,7 +487,7 @@ The set of acceptable TYPEs (also called \"specializers\") 
is defined
                (or (not (fboundp 'byte-compile-warning-enabled-p))
                    (byte-compile-warning-enabled-p 'obsolete name))
                (let* ((obsolete (get name 'byte-obsolete-info)))
-                 (macroexp--warn-and-return
+                 (macroexp-warn-and-return
                   (macroexp--obsolete-warning name obsolete "generic function")
                   nil)))
          ;; You could argue that `defmethod' modifies rather than defines the
diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el
index b852d82..007466b 100644
--- a/lisp/emacs-lisp/cl-macs.el
+++ b/lisp/emacs-lisp/cl-macs.el
@@ -565,7 +565,7 @@ its argument list allows full Common Lisp conventions."
                              ,(length (cl-ldiff args p)))
                  exactarg (not (eq args p)))))
       (while (and args (not (memq (car args) cl--lambda-list-keywords)))
-       (let ((poparg (list (if (or (cdr args) (not exactarg)) 'pop 'car)
+       (let ((poparg (list (if (or (cdr args) (not exactarg)) 'pop 'car-safe)
                            restarg)))
          (cl--do-arglist
           (pop args)
@@ -2393,7 +2393,7 @@ by EXPANSION, and (setq NAME ...) will act like (setf 
EXPANSION ...).
                                                (append bindings venv))
                                          macroexpand-all-environment))))
             (if malformed-bindings
-                (macroexp--warn-and-return
+                (macroexp-warn-and-return
                  (format-message "Malformed `cl-symbol-macrolet' binding(s): 
%S"
                                  (nreverse malformed-bindings))
                  expansion)
@@ -3032,7 +3032,7 @@ Supported keywords for slots are:
                     forms)
               (when (cl-oddp (length desc))
                 (push
-                 (macroexp--warn-and-return
+                 (macroexp-warn-and-return
                   (format "Missing value for option `%S' of slot `%s' in 
struct %s!"
                           (car (last desc)) slot name)
                   'nil)
@@ -3041,7 +3041,7 @@ Supported keywords for slots are:
                            (not (keywordp (car desc))))
                   (let ((kw (car defaults)))
                     (push
-                     (macroexp--warn-and-return
+                     (macroexp-warn-and-return
                       (format "  I'll take `%s' to be an option rather than a 
default value."
                               kw)
                       'nil)
diff --git a/lisp/emacs-lisp/eieio-core.el b/lisp/emacs-lisp/eieio-core.el
index a8361c0..e7727fd 100644
--- a/lisp/emacs-lisp/eieio-core.el
+++ b/lisp/emacs-lisp/eieio-core.el
@@ -729,7 +729,7 @@ Argument FN is the function calling this verifier."
               (pcase slot
                 ((and (or `',name (and name (pred keywordp)))
                       (guard (not (memq name eieio--known-slot-names))))
-                 (macroexp--warn-and-return
+                 (macroexp-warn-and-return
                   (format-message "Unknown slot `%S'" name) exp 'compile-only))
                 (_ exp))))
            (gv-setter eieio-oset))
diff --git a/lisp/emacs-lisp/eieio.el b/lisp/emacs-lisp/eieio.el
index d3e5d03..910023b 100644
--- a/lisp/emacs-lisp/eieio.el
+++ b/lisp/emacs-lisp/eieio.el
@@ -269,7 +269,7 @@ This method is obsolete."
                        (lambda (whole)
                          (if (not (stringp (car slots)))
                              whole
-                           (macroexp--warn-and-return
+                           (macroexp-warn-and-return
                             (format "Obsolete name arg %S to constructor %S"
                                     (car slots) (car whole))
                             ;; Keep the name arg, for backward compatibility,
diff --git a/lisp/emacs-lisp/gv.el b/lisp/emacs-lisp/gv.el
index 2b213e2..3d80549 100644
--- a/lisp/emacs-lisp/gv.el
+++ b/lisp/emacs-lisp/gv.el
@@ -593,7 +593,7 @@ binding mode."
             ;; dynamic binding mode as well.
             (eq (car-safe code) 'cons))
         code
-      (macroexp--warn-and-return
+      (macroexp-warn-and-return
        "Use of gv-ref probably requires lexical-binding"
        code))))
 
diff --git a/lisp/emacs-lisp/inline.el b/lisp/emacs-lisp/inline.el
index d6106fe..36d71a8 100644
--- a/lisp/emacs-lisp/inline.el
+++ b/lisp/emacs-lisp/inline.el
@@ -262,7 +262,7 @@ See Info node `(elisp)Defining Functions' for more details."
   '(throw 'inline--just-use
           ;; FIXME: This would inf-loop by calling us right back when
           ;; macroexpand-all recurses to expand inline--form.
-          ;; (macroexp--warn-and-return (format ,@args)
+          ;; (macroexp-warn-and-return (format ,@args)
           ;;                            inline--form)
           inline--form))
 
diff --git a/lisp/emacs-lisp/macroexp.el b/lisp/emacs-lisp/macroexp.el
index d52aee5..4d04bfa 100644
--- a/lisp/emacs-lisp/macroexp.el
+++ b/lisp/emacs-lisp/macroexp.el
@@ -135,28 +135,33 @@ Other uses risk returning non-nil value that point to the 
wrong file."
 
 (defvar macroexp--warned (make-hash-table :test #'equal :weakness 'key))
 
-(defun macroexp--warn-and-return (msg form &optional compile-only)
+(defun macroexp--warn-wrap (msg form)
   (let ((when-compiled (lambda () (byte-compile-warn "%s" msg))))
-    (cond
-     ((null msg) form)
-     ((macroexp-compiling-p)
-      (if (and (consp form) (gethash form macroexp--warned))
-          ;; Already wrapped this exp with a warning: avoid inf-looping
-          ;; where we keep adding the same warning onto `form' because
-          ;; macroexpand-all gets right back to macroexpanding `form'.
-          form
-        (puthash form form macroexp--warned)
-        `(progn
-           (macroexp--funcall-if-compiled ',when-compiled)
-           ,form)))
-     (t
-      (unless compile-only
-        (message "%sWarning: %s"
-                 (if (stringp load-file-name)
-                     (concat (file-relative-name load-file-name) ": ")
-                   "")
-                 msg))
-      form))))
+    `(progn
+       (macroexp--funcall-if-compiled ',when-compiled)
+       ,form)))
+
+(define-obsolete-function-alias 'macroexp--warn-and-return
+  #'macroexp-warn-and-return "28.1")
+(defun macroexp-warn-and-return (msg form &optional compile-only)
+  (cond
+   ((null msg) form)
+   ((macroexp-compiling-p)
+    (if (and (consp form) (gethash form macroexp--warned))
+        ;; Already wrapped this exp with a warning: avoid inf-looping
+        ;; where we keep adding the same warning onto `form' because
+        ;; macroexpand-all gets right back to macroexpanding `form'.
+        form
+      (puthash form form macroexp--warned)
+      (macroexp--warn-wrap msg form)))
+   (t
+    (unless compile-only
+      (message "%sWarning: %s"
+               (if (stringp load-file-name)
+                   (concat (file-relative-name load-file-name) ": ")
+                 "")
+               msg))
+    form)))
 
 (defun macroexp--obsolete-warning (fun obsolescence-data type)
   (let ((instead (car obsolescence-data))
@@ -205,7 +210,7 @@ Other uses risk returning non-nil value that point to the 
wrong file."
                  (byte-compile-warning-enabled-p 'obsolete (car form))))
         (let* ((fun (car form))
                (obsolete (get fun 'byte-obsolete-info)))
-          (macroexp--warn-and-return
+          (macroexp-warn-and-return
            (macroexp--obsolete-warning
             fun obsolete
             (if (symbolp (symbol-function fun))
@@ -260,7 +265,7 @@ Other uses risk returning non-nil value that point to the 
wrong file."
                    values (cdr values))))
       (setq arglist (cdr arglist)))
     (if values
-        (macroexp--warn-and-return
+        (macroexp-warn-and-return
          (format (if (eq values 'too-few)
                      "attempt to open-code `%s' with too few arguments"
                    "attempt to open-code `%s' with too many arguments")
@@ -314,7 +319,7 @@ Assumes the caller has bound `macroexpand-all-environment'."
                        (macroexp--cons (macroexp--all-clauses bindings 1)
                                        (if (null body)
                                            (macroexp-unprogn
-                                            (macroexp--warn-and-return
+                                            (macroexp-warn-and-return
                                              (format "Empty %s body" fun)
                                              nil t))
                                          (macroexp--all-forms body))
@@ -344,13 +349,13 @@ Assumes the caller has bound 
`macroexpand-all-environment'."
       ;; First arg is a function:
       (`(,(and fun (or 'funcall 'apply 'mapcar 'mapatoms 'mapconcat 'mapc))
          ',(and f `(lambda . ,_)) . ,args)
-       (macroexp--warn-and-return
+       (macroexp-warn-and-return
         (format "%s quoted with ' rather than with #'"
                 (list 'lambda (nth 1 f) '...))
         (macroexp--expand-all `(,fun #',f . ,args))))
       ;; Second arg is a function:
       (`(,(and fun (or 'sort)) ,arg1 ',(and f `(lambda . ,_)) . ,args)
-       (macroexp--warn-and-return
+       (macroexp-warn-and-return
         (format "%s quoted with ' rather than with #'"
                 (list 'lambda (nth 1 f) '...))
         (macroexp--expand-all `(,fun ,arg1 #',f . ,args))))
diff --git a/lisp/emacs-lisp/pcase.el b/lisp/emacs-lisp/pcase.el
index c7288b7..95e5dd3 100644
--- a/lisp/emacs-lisp/pcase.el
+++ b/lisp/emacs-lisp/pcase.el
@@ -469,8 +469,10 @@ for the result of evaluating EXP (first arg to `pcase').
 ;; the depth of the generated tree.
 (defun pcase--if (test then else)
   (cond
-   ((eq else :pcase--dontcare) then)
-   ((eq then :pcase--dontcare) (debug) else) ;Can/should this ever happen?
+   ((eq else :pcase--dontcare) `(progn (ignore ,test) ,then))
+   ;; This happens very rarely.  Known case:
+   ;;     (pcase EXP ((and 1 pcase--dontcare) FOO))
+   ((eq then :pcase--dontcare) `(progn (ignore ,test) ,else))
    (t (macroexp-if test then else))))
 
 ;; Note about MATCH:
@@ -845,7 +847,7 @@ Otherwise, it defers to REST which is a list of branches of 
the form
        ((memq upat '(t _))
         (let ((code (pcase--u1 matches code vars rest)))
           (if (eq upat '_) code
-            (macroexp--warn-and-return
+            (macroexp-warn-and-return
              "Pattern t is deprecated.  Use `_' instead"
              code))))
        ((eq upat 'pcase--dontcare) :pcase--dontcare)
@@ -971,8 +973,8 @@ The predicate is the logical-AND of:
               (nreverse upats))))
    ((consp qpat)
     `(and (pred consp)
-          (app car ,(list '\` (car qpat)))
-          (app cdr ,(list '\` (cdr qpat)))))
+          (app car-safe ,(list '\` (car qpat)))
+          (app cdr-safe ,(list '\` (cdr qpat)))))
    ((or (stringp qpat) (numberp qpat) (symbolp qpat)) `',qpat)
    ;; In all other cases just raise an error so we can't break
    ;; backward compatibility when adding \` support for other
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index 20c7f20..37bed0c 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1406,6 +1406,7 @@ which see."
   (interactive "P")
   (cond (edebug-it
         (require 'edebug)
+        (defvar edebug-all-defs)
         (eval-defun (not edebug-all-defs)))
        (t
         (if (null eval-expression-debug-on-error)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]