GNU bug report logs - #23530
[PROPOSED PATCH] ‘make check-declare’ now chatters less

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23530 in the body.
You can then email your comments to 23530 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Cc: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: [PROPOSED PATCH] ‘make check-declare’ now chatters less
Date: Fri, 13 May 2016 08:44:55 -0700

* etc/NEWS: Document this.
* lisp/emacs-lisp/check-declare.el (check-declare-locate):
Return relative names, not absolute.
(check-declare-scan, check-declare-verify, check-declare-warn)
(check-declare-file, check-declare-directory):
Generate less chatter.  Use relative file names rather than
absolute.  Don’t give up on computing a good file name for a
diagnostic merely because the function name was bad.  Make
malformed declarations more noticeable.  Don’t warn about
"ext:..." declarations if check-declare-ext-errors is nil.
(check-declare-errmsg): Remove.
(check-declare-warn): New optional arg LINE.
(check-declare-files): Put status into mode line rather than
chattering.
---
 etc/NEWS                         |   4 ++
 lisp/emacs-lisp/check-declare.el | 142 ++++++++++++++++-----------------------
 2 files changed, 63 insertions(+), 83 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 31229f1..b0bdc86 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -370,6 +370,10 @@ of curved quotes in format arguments to functions like 'message' and
 'format-message'.  In particular, when this variable's value is
 'grave', all quotes in formats are output as-is.
 
+** Functions like 'check-declare-file' and 'check-declare-directory'
+now generate less chatter and more-compact diagnostics.  The auxiliary
+function 'check-declare-errmsg' has been removed.
+
 
 * Lisp Changes in Emacs 25.2
 
diff --git a/lisp/emacs-lisp/check-declare.el b/lisp/emacs-lisp/check-declare.el
index bc7b5ae..e1e756b 100644
--- a/lisp/emacs-lisp/check-declare.el
+++ b/lisp/emacs-lisp/check-declare.el
@@ -43,7 +43,7 @@ check-declare-warning-buffer
   "Name of buffer used to display any `check-declare' warnings.")
 
 (defun check-declare-locate (file basefile)
-  "Return the full path of FILE.
+  "Return the relative name of FILE.
 Expands files with a \".c\" or \".m\" extension relative to the Emacs
 \"src/\" directory.  Otherwise, `locate-library' searches for FILE.
 If that fails, expands FILE relative to BASEFILE's directory part.
@@ -70,6 +70,7 @@ check-declare-locate
                       (string-match "\\.el\\'" tfile))
                   tfile
                 (concat tfile ".el")))))
+    (setq file (file-relative-name file))
     (if ext (concat "ext:" file)
       file)))
 
@@ -80,49 +81,40 @@ check-declare-scan
 defines FN, with ARGLIST.  FILEONLY non-nil means only check that FNFILE
 exists, not that it defines FN.  This is for function definitions that we
 don't know how to recognize (e.g. some macros)."
-  (let ((m (format "Scanning %s..." file))
-        alist form len fn fnfile arglist fileonly)
-    (message "%s" m)
+  (let (alist)
     (with-temp-buffer
       (insert-file-contents file)
       ;; FIXME we could theoretically be inside a string.
       (while (re-search-forward "^[ \t]*\\((declare-function\\)[ \t\n]" nil t)
-        (goto-char (match-beginning 1))
-        (if (and (setq form (ignore-errors (read (current-buffer))))
+        (let ((pos (match-beginning 1)))
+          (goto-char pos)
+          (let ((form (ignore-errors (read (current-buffer))))
+                len fn formfile fnfile arglist fileonly)
+            (if (and
                  ;; Exclude element of byte-compile-initial-macro-environment.
                  (or (listp (cdr form)) (setq form nil))
                  (> (setq len (length form)) 2)
                  (< len 6)
+                 (setq formfile (nth 2 form))
                  (symbolp (setq fn (cadr form)))
                  (setq fn (symbol-name fn)) ; later we use as a search string
-                 (stringp (setq fnfile (nth 2 form)))
-                 (setq fnfile (check-declare-locate fnfile
-                                                    (expand-file-name file)))
+                 (stringp formfile)
+                 (setq fnfile (check-declare-locate formfile file))
                  ;; Use t to distinguish unspecified arglist from empty one.
                  (or (eq t (setq arglist (if (> len 3)
                                              (nth 3 form)
                                            t)))
                      (listp arglist))
                  (symbolp (setq fileonly (nth 4 form))))
-            (setq alist (cons (list fnfile fn arglist fileonly) alist))
-          ;; FIXME make this more noticeable.
-          (if form (message "Malformed declaration for `%s'" (cadr form))))))
-    (message "%sdone" m)
+                (setq alist (cons (list fnfile fn arglist fileonly) alist))
+              (when form
+                (check-declare-warn file (or fn "unknown function")
+                                    (if (stringp formfile) formfile
+                                      "unknown file")
+                                    "Malformed declaration"
+                                    (line-number-at-pos pos))))))))
     alist))
 
-(defun check-declare-errmsg (errlist &optional full)
-  "Return a string with the number of errors in ERRLIST, if any.
-Normally just counts the number of elements in ERRLIST.
-With optional argument FULL, sums the number of elements in each element."
-  (if errlist
-      (let ((l (length errlist)))
-        (when full
-          (setq l 0)
-          (dolist (e errlist)
-            (setq l (+ l (1- (length e))))))
-        (format "%d problem%s found" l (if (= l 1) "" "s")))
-    "OK"))
-
 (autoload 'byte-compile-arglist-signature "bytecomp")
 
 (defgroup check-declare nil
@@ -144,11 +136,9 @@ check-declare-verify
 Returns nil if all claims are found to be true, otherwise a list
 of errors with elements of the form \(FILE FN TYPE), where TYPE
 is a string giving details of the error."
-  (let ((m (format "Checking %s..." fnfile))
-        (cflag (member (file-name-extension fnfile) '("c" "m")))
+  (let ((cflag (member (file-name-extension fnfile) '("c" "m")))
         (ext (string-match "^ext:" fnfile))
         re fn sig siglist arglist type errlist minargs maxargs)
-    (message "%s" m)
     (if ext
         (setq fnfile (substring fnfile 4)))
     (if (file-regular-p fnfile)
@@ -216,7 +206,8 @@ check-declare-verify
       (setq arglist (nth 2 e)
             type
             (if (not re)
-                "file not found"
+                (when (or check-declare-ext-errors (not ext))
+                  "file not found")
               (if (not (setq sig (assoc (cadr e) siglist)))
                   (unless (nth 3 e)     ; fileonly
                     "function not found")
@@ -235,13 +226,6 @@ check-declare-verify
                        "arglist mismatch")))))
       (when type
         (setq errlist (cons (list (car e) (cadr e) type) errlist))))
-    (message "%s%s" m
-             (if (or re (or check-declare-ext-errors
-                            (not ext)))
-                 (check-declare-errmsg errlist)
-               (progn
-                 (setq errlist nil)
-                 "skipping external file")))
     errlist))
 
 (defun check-declare-sort (alist)
@@ -258,30 +242,27 @@ check-declare-sort
           (setq sort (cons (list fnfile (cons file rest)) sort)))))
     sort))
 
-(defun check-declare-warn (file fn fnfile type)
+(defun check-declare-warn (file fn fnfile type &optional line)
   "Warn that FILE made a false claim about FN in FNFILE.
-TYPE is a string giving the nature of the error.  Warning is displayed in
-`check-declare-warning-buffer'."
+TYPE is a string giving the nature of the error.
+Optional LINE is the claim's line number; otherwise, search for the claim.
+Display warning in `check-declare-warning-buffer'."
   (let ((warning-prefix-function
          (lambda (level entry)
-           (let ((line 0)
-                 (col 0))
-             (insert
-              (with-current-buffer (find-file-noselect file)
-                (goto-char (point-min))
-                (when (re-search-forward
-                       (format "(declare-function[ \t\n]+%s" fn) nil t)
-                  (goto-char (match-beginning 0))
-                  (setq line (line-number-at-pos))
-                  (setq col (1+ (current-column))))
-                (format "%s:%d:%d:"
-                        (file-name-nondirectory file)
-                        line col))))
+	   (insert (format "%s:%d:" (file-relative-name file) (or line 0)))
            entry))
         (warning-fill-prefix "    "))
+    (unless line
+      (with-current-buffer (find-file-noselect file)
+        (goto-char (point-min))
+        (when (and (not line)
+                   (re-search-forward
+                    (format "(declare-function[ \t\n]+%s" fn) nil t))
+          (goto-char (match-beginning 0))
+          (setq line (line-number-at-pos)))))
     (display-warning 'check-declare
                      (format-message "said `%s' was defined in %s: %s"
-                                     fn (file-name-nondirectory fnfile) type)
+                                     fn (file-relative-name fnfile) type)
                      nil check-declare-warning-buffer)))
 
 (declare-function compilation-forget-errors "compile" ())
@@ -289,7 +270,18 @@ check-declare-warn
 (defun check-declare-files (&rest files)
   "Check veracity of all `declare-function' statements in FILES.
 Return a list of any errors found."
-  (let (alist err errlist)
+  (if (get-buffer check-declare-warning-buffer)
+      (kill-buffer check-declare-warning-buffer))
+  (let ((buf (get-buffer-create check-declare-warning-buffer))
+        alist err errlist)
+    (with-current-buffer buf
+      (unless (derived-mode-p 'compilation-mode)
+        (compilation-mode))
+      (setq mode-line-process
+            '(:propertize ":run" face compilation-mode-line-run))
+      (let ((inhibit-read-only t))
+        (insert "\f\n"))
+      (compilation-forget-errors))
     (dolist (file files)
       (setq alist (cons (cons file (check-declare-scan file)) alist)))
     ;; Sort so that things are ordered by the files supposed to
@@ -298,19 +290,15 @@ check-declare-files
       (if (setq err (check-declare-verify (car e) (cdr e)))
           (setq errlist (cons (cons (car e) err) errlist))))
     (setq errlist (nreverse errlist))
-    (if (get-buffer check-declare-warning-buffer)
-        (kill-buffer check-declare-warning-buffer))
-    (with-current-buffer (get-buffer-create check-declare-warning-buffer)
-      (unless (derived-mode-p 'compilation-mode)
-        (compilation-mode))
-      (let ((inhibit-read-only t))
-        (insert "\f\n"))
-      (compilation-forget-errors))
     ;; Sort back again so that errors are ordered by the files
     ;; containing the declare-function statements.
     (dolist (e (check-declare-sort errlist))
       (dolist (f (cdr e))
         (check-declare-warn (car e) (cadr f) (car f) (nth 2 f))))
+    (with-current-buffer buf
+      (setq mode-line-process
+            '(:propertize ":exit" face compilation-mode-line-run))
+      (force-mode-line-update))
     errlist))
 
 ;;;###autoload
@@ -320,34 +308,22 @@ check-declare-file
   (interactive "fFile to check: ")
   (or (file-exists-p file)
       (error "File `%s' not found" file))
-  (let ((m (format "Checking %s..." file))
-        errlist)
-    (message "%s" m)
-    (setq errlist (check-declare-files file))
-    (message "%s%s" m (check-declare-errmsg errlist))
-    errlist))
+  (check-declare-files file))
 
 ;;;###autoload
 (defun check-declare-directory (root)
   "Check veracity of all `declare-function' statements under directory ROOT.
 Returns non-nil if any false statements are found."
   (interactive "DDirectory to check: ")
-  (or (file-directory-p (setq root (expand-file-name root)))
+  (setq root (directory-file-name (file-relative-name root)))
+  (or (file-directory-p root)
       (error "Directory `%s' not found" root))
-  (let ((m "Checking `declare-function' statements...")
-        (m2 "Finding files with declarations...")
-        errlist files)
-    (message "%s" m)
-    (message "%s" m2)
-    (setq files (process-lines find-program root
-			       "-name" "*.el"
-			       "-exec" grep-program
-			       "-l" "^[ \t]*(declare-function" "{}" "+"))
-    (message "%s%d found" m2 (length files))
+  (let ((files (process-lines find-program root
+                              "-name" "*.el"
+                              "-exec" grep-program
+                              "-l" "^[ \t]*(declare-function" "{}" "+")))
     (when files
-      (setq errlist (apply 'check-declare-files files))
-      (message "%s%s" m (check-declare-errmsg errlist t))
-      errlist)))
+      (apply #'check-declare-files files))))
 
 (provide 'check-declare)
 
-- 
2.5.5

Message #8 received at 23530 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 23530 <at> debbugs.gnu.org
Subject: Re: [PROPOSED PATCH] ‘make check-declare’ now chatters less
Date: Fri, 13 May 2016 09:22:13 -0700

[Message part 1 (text/plain, inline)]

To give people a feel for the effect of this patch, attached is the 
output of ‘make check-declare’ before and after the patch. The patch 
shrinks the output size from 148 to 8 kB.

[check-declare0.txt (text/plain, attachment)]

[check-declare1.txt (text/plain, attachment)]

Message #13 received at 23530-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 23530-done <at> debbugs.gnu.org
Subject: Re: [PROPOSED PATCH] ‘make check-declare’ now chatters less
Date: Wed, 18 May 2016 00:50:35 -0700

No further comment, so installed this patch into master and am closing the bug
report.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.