Package: emacs;
Reported by: Pip Cet <pipcet <at> protonmail.com>
Date: Sat, 7 Jun 2025 14:27:02 UTC
Severity: normal
Found in version 31.0.50
To reply to this bug, email your comments to 78716 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sat, 07 Jun 2025 14:27:02 GMT) Full text and rfc822 format available.Pip Cet <pipcet <at> protonmail.com>
:bug-gnu-emacs <at> gnu.org
.
(Sat, 07 Jun 2025 14:27:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Pip Cet <pipcet <at> protonmail.com> To: bug-gnu-emacs <at> gnu.org Subject: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 07 Jun 2025 14:25:47 +0000
This is a spin-off of bug#78704, which is about marking more functions as having an important return value. We do not currently warn consistently when discarding the value of a function call to a side-effect-free function, even though the return value is usually important. It's not clear that we should, but I tried enabling such warnings and found ~5 questionable places in the code in lisp/. 1. We don't ever warn about side-effect-and-error-free functions like "not"; this seems to me to be strange behavior on the part of the byte compiler, but it's probably been that way for a long time. For example, this "not" in an implicit progn, in vc/vc.el: (insert-string (format "Changes to %s since last lock:\n\n" file)) (not (beep)) (yes-or-no-p (concat "File has unlocked changes, " "claim lock retaining changes? "))) That's commit bbf97570e5037b2417c9dd6353e1b2a9afadda7c, from 1993, moved around in 2000 but essentially untouched. Has the byte compiler been silent on this situation for all this time? Is it worth changing? 2. We also don't warn about function calls which we optimize away; this also seems problematic if the form we optimize away was present in the original source code, but it's easier to write macros which generate intermediate forms which can then be optimized, and I don't think we can currently tell whether we're looking at macro-expanded code or not. For example, my naive experiment produced false warnings when (pop list2) appeared in a context in which its return value was ignored, which is a common idiom and probably accounts for the majority of the warnings produced. OTOH, code like this, in cedet/semantic/find.el: (defun semantic-brute-find-first-tag-by-name (name streamorbuffer &optional search-parts search-include) "Find a tag NAME within STREAMORBUFFER. NAME is a string. If SEARCH-PARTS is non-nil, search children of tags. If SEARCH-INCLUDE was never implemented. Respects `semantic-case-fold'. Use `semantic-find-first-tag-by-name' instead." (let* ((stream (semantic-something-to-tag-table streamorbuffer)) (m (assoc-string name stream semantic-case-fold))) (if m m (let ((toklst stream) (children nil)) (while (and (not m) toklst) (if search-parts (progn (setq children (semantic-tag-components-with-overlays (car toklst))) (if children (setq m (semantic-brute-find-first-tag-by-name name children search-parts search-include))))) (setq toklst (cdr toklst))) (if (not m) ;; Go to dependencies, and search there. nil) m)))) probably should generate a warning, because the if form seems completely mysterious to me. And this code in cc-engine.el: (and (c-syntactic-re-search-forward "[;={]" start t t t) (eq (char-before) ?=) c-overloadable-operators-regexp c-opt-op-identifier-prefix (save-excursion (eq (c-backward-token-2) 0) (looking-at c-overloadable-operators-regexp) (eq (c-backward-token-2) 0) (looking-at c-opt-op-identifier-prefix)))) seems like what might have been intended was another (and ...) in (save-excursion ...), but I'm not entirely sure. My suggestion is to fix the first issue and complain about such forms, by doing something like this: diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 5fa65ff71a6..4716e150faf 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3470,26 +3470,25 @@ byte-compile-form (when byte-compile--for-effect (let ((sef (function-get (car form) 'side-effect-free))) - (cond - ((and sef (or (eq sef 'error-free) - byte-compile-delete-errors)) - ;; This transform is normally done in the Lisp optimizer, - ;; so maybe we don't need to bother about it here? - (setq form (cons 'progn (cdr form))) - (setq handler #'byte-compile-progn)) - ((and (or sef (function-get (car form) 'important-return-value)) - ;; Don't warn for arguments to `ignore'. - (not (eq byte-compile--for-effect 'for-effect-no-warn)) - (bytecomp--actually-important-return-value-p form) - (byte-compile-warning-enabled-p - 'ignored-return-value (car form))) - (byte-compile-warn-x - (car form) - "value from call to `%s' is unused%s" - (car form) - (cond ((eq (car form) 'mapcar) - "; use `mapc' or `dolist' instead") - (t ""))))))) + (and (or sef (function-get (car form) 'important-return-value)) + ;; Don't warn for arguments to `ignore'. + (not (eq byte-compile--for-effect 'for-effect-no-warn)) + (bytecomp--actually-important-return-value-p form) + (byte-compile-warning-enabled-p + 'ignored-return-value (car form)) + (byte-compile-warn-x + (car form) + "value from call to `%s' is unused%s" + (car form) + (cond ((eq (car form) 'mapcar) + "; use `mapc' or `dolist' instead") + (t "")))) + (and sef (or (eq sef 'error-free) + byte-compile-delete-errors) + ;; This transform is normally done in the Lisp optimizer, + ;; so maybe we don't need to bother about it here? + (setq form (cons 'progn (cdr form))) + (setq handler #'byte-compile-progn)))) (if (and handler ;; Make sure that function exists. The second class of warnings should not be enabled by default, but maybe it can become an 'extra warning level which we run once in a while? diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el index 652c79e9c93..4e1e7cd7caf 100644 --- a/lisp/emacs-lisp/byte-opt.el +++ b/lisp/emacs-lisp/byte-opt.el @@ -486,6 +486,15 @@ byte-optimize-form-code-walker (if-let* ((tmp (byte-opt--fget fn 'side-effect-free))) (or byte-compile-delete-errors (eq tmp 'error-free))))) + (and + (not (eq for-effect 'for-effect-no-warn)) + (bytecomp--actually-important-return-value-p form) + (byte-compile-warning-enabled-p + 'ignored-return-value (car form)) + (byte-compile-warn-x + fn + "value from call to `%s' is unused" + fn)) (byte-compile-log " %s called for effect; deleted" fn) (byte-optimize-form (cons 'progn (cdr form)) t)) But I think to decide that, we need to look at the potential bugs caught by this code (changed to what I think what was meant): diff --git a/lisp/which-key.el b/lisp/which-key.el index 0be0feb542e..3b339a14bf8 100644 --- a/lisp/which-key.el +++ b/lisp/which-key.el @@ -2464,7 +2464,7 @@ which-key-dump-bindings (let* ((buffer (get-buffer-create buffer-name)) (keys (which-key--get-bindings (kbd prefix)))) (with-current-buffer buffer - (point-max) + (goto-char (point-max)) (save-excursion (dolist (key keys) (insert (apply #'format "%s%s%s\n" key))))) diff --git a/lisp/international/quail.el b/lisp/international/quail.el index 2b66a0bc0f7..a64a86df91e 100644 --- a/lisp/international/quail.el +++ b/lisp/international/quail.el @@ -2113,7 +2113,7 @@ quail-update-guidance nil ;; We want to make both KEY and STR visible, but if the ;; window is too short, make at least STR visible. - (setq pos (progn (point) (goto-char pos))) + (setq pos (prog1 (point) (goto-char pos))) (beginning-of-line) (set-window-start win (point)) (if (not (pos-visible-in-window-p pos win)) diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el index 59e1b49f92c..0c358724a39 100644 --- a/lisp/org/org-table.el +++ b/lisp/org/org-table.el @@ -1930,8 +1930,8 @@ org-table-copy-down (setq field-above (let ((f (unless (= beg (line-beginning-position)) (forward-line -1) - (not (org-at-table-hline-p)) - (org-table-get-field column)))) + (and (not (org-at-table-hline-p)) + (org-table-get-field column))))) (and (org-string-nw-p f) (org-trim f))))) ;; Compute next field. diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el index d9044c03aea..5d4137aa7fa 100644 --- a/lisp/progmodes/ruby-mode.el +++ b/lisp/progmodes/ruby-mode.el @@ -1782,8 +1782,8 @@ ruby-forward-sexp (nth 1 (setq state (apply #'ruby-parse-partial nil state)))) (setq expr t) - (skip-chars-forward "<")) - (not expr)))) + (skip-chars-forward "<") + (not expr))))) (setq i (1- i))) ((error) (forward-word-strictly 1))) i)))) diff --git a/lisp/progmodes/vhdl-mode.el b/lisp/progmodes/vhdl-mode.el index 593a83ceffa..0bdb877a10f 100644 --- a/lisp/progmodes/vhdl-mode.el +++ b/lisp/progmodes/vhdl-mode.el @@ -7133,8 +7133,9 @@ vhdl-get-syntactic-context ((and (save-excursion (goto-char (1+ containing-sexp)) (skip-chars-forward " \t") - (not (eolp)) - (not (looking-at "--\\|`"))) + (and + (not (eolp)) + (not (looking-at "--\\|`")))) (save-excursion (vhdl-beginning-of-statement-1 containing-sexp) (skip-chars-backward " \t(") ;; Note: `static-if' can be copied into a package to enable it to be ;; used in Emacsen older than Emacs 30.1. If the package is used in Plus some unnecessary but apparently harmless code: diff --git a/lisp/gnus/gnus-score.el b/lisp/gnus/gnus-score.el index a4102ef0b51..54f07b9d116 100644 --- a/lisp/gnus/gnus-score.el +++ b/lisp/gnus/gnus-score.el @@ -1489,7 +1489,6 @@ gnus-score-save (setq gnus-score-alist nil) (nnheader-set-temp-buffer " *Gnus Scores*") (while cache - (current-buffer) (setq entry (pop cache) file (nnheader-translate-file-chars (car entry) t) score (cdr entry)) diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el index 74dff3217ff..7e27019bfb4 100644 --- a/lisp/progmodes/gdb-mi.el +++ b/lisp/progmodes/gdb-mi.el @@ -1643,7 +1643,6 @@ gdb-buffer-type (defun gdb-buffer-shows-main-thread-p () "Return t if current GDB buffer shows main selected thread and is not bound to it." - (current-buffer) (not (local-variable-p 'gdb-thread-number))) (defun gdb-get-buffer (buffer-type &optional thread) diff --git a/lisp/textmodes/table.el b/lisp/textmodes/table.el index 0e024f9261e..3e6954256a6 100644 --- a/lisp/textmodes/table.el +++ b/lisp/textmodes/table.el @@ -5360,10 +5360,8 @@ table--goto-coordinate (if (/= (move-to-column x) x) (if (> (move-to-column x) x) (if no-tab-expansion - (progn - (while (> (move-to-column x) x) - (setq x (1- x))) - (point)) + (while (> (move-to-column x) x) + (setq x (1- x))) (throw 'exit (move-to-column x t))) (throw 'exit nil))) (move-to-column x t)) diff --git a/lisp/speedbar.el b/lisp/speedbar.el index b1fd141321c..2b54c456651 100644 --- a/lisp/speedbar.el +++ b/lisp/speedbar.el @@ -1232,8 +1232,7 @@ speedbar-mode dframe-mouse-click-function #'speedbar-click dframe-mouse-position-function #'speedbar-position-cursor-on-line) (setq-local tab-bar-mode nil) - (setq tab-line-exclude nil)) - speedbar-buffer) + (setq tab-line-exclude nil))) (define-obsolete-function-alias 'speedbar-message 'dframe-message "24.4") So five potentially user-visible bugs in the current Emacs code base, plenty of false positives for the second change, at least.
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sat, 28 Jun 2025 09:10:01 GMT) Full text and rfc822 format available.Message #8 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Pip Cet <pipcet <at> protonmail.com>, Stefan Monnier <monnier <at> iro.umontreal.ca>, Mattias Engdegård <mattiase <at> acm.org> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 28 Jun 2025 12:09:06 +0300
> Date: Sat, 07 Jun 2025 14:25:47 +0000 > From: Pip Cet via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> > > This is a spin-off of bug#78704, which is about marking more functions > as having an important return value. > > We do not currently warn consistently when discarding the value of a > function call to a side-effect-free function, even though the return > value is usually important. It's not clear that we should, but I tried > enabling such warnings and found ~5 questionable places in the code in > lisp/. > > 1. We don't ever warn about side-effect-and-error-free functions like > "not"; this seems to me to be strange behavior on the part of the byte > compiler, but it's probably been that way for a long time. > > For example, this "not" in an implicit progn, in vc/vc.el: > > (insert-string (format "Changes to %s since last lock:\n\n" > file)) > (not (beep)) > (yes-or-no-p > (concat "File has unlocked changes, " > "claim lock retaining changes? "))) > > That's commit bbf97570e5037b2417c9dd6353e1b2a9afadda7c, from 1993, moved > around in 2000 but essentially untouched. Has the byte compiler been > silent on this situation for all this time? Is it worth changing? > > 2. We also don't warn about function calls which we optimize away; this > also seems problematic if the form we optimize away was present in the > original source code, but it's easier to write macros which generate > intermediate forms which can then be optimized, and I don't think we can > currently tell whether we're looking at macro-expanded code or not. > > For example, my naive experiment produced false warnings when > > (pop list2) > > appeared in a context in which its return value was ignored, which is a > common idiom and probably accounts for the majority of the warnings > produced. > > OTOH, code like this, in cedet/semantic/find.el: > > (defun semantic-brute-find-first-tag-by-name > (name streamorbuffer &optional search-parts search-include) > "Find a tag NAME within STREAMORBUFFER. NAME is a string. > If SEARCH-PARTS is non-nil, search children of tags. > If SEARCH-INCLUDE was never implemented. > Respects `semantic-case-fold'. > > Use `semantic-find-first-tag-by-name' instead." > (let* ((stream (semantic-something-to-tag-table streamorbuffer)) > (m (assoc-string name stream semantic-case-fold))) > (if m > m > (let ((toklst stream) > (children nil)) > (while (and (not m) toklst) > (if search-parts > (progn > (setq children (semantic-tag-components-with-overlays > (car toklst))) > (if children > (setq m (semantic-brute-find-first-tag-by-name > name children search-parts search-include))))) > (setq toklst (cdr toklst))) > (if (not m) > ;; Go to dependencies, and search there. > nil) > m)))) > > probably should generate a warning, because the if form seems completely > mysterious to me. > > And this code in cc-engine.el: > > (and (c-syntactic-re-search-forward "[;={]" start t t t) > (eq (char-before) ?=) > c-overloadable-operators-regexp > c-opt-op-identifier-prefix > (save-excursion > (eq (c-backward-token-2) 0) > (looking-at c-overloadable-operators-regexp) > (eq (c-backward-token-2) 0) > (looking-at c-opt-op-identifier-prefix)))) > > seems like what might have been intended was another (and ...) in > (save-excursion ...), but I'm not entirely sure. > > My suggestion is to fix the first issue and complain about such forms, > by doing something like this: > > diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el > index 5fa65ff71a6..4716e150faf 100644 > --- a/lisp/emacs-lisp/bytecomp.el > +++ b/lisp/emacs-lisp/bytecomp.el > @@ -3470,26 +3470,25 @@ byte-compile-form > > (when byte-compile--for-effect > (let ((sef (function-get (car form) 'side-effect-free))) > - (cond > - ((and sef (or (eq sef 'error-free) > - byte-compile-delete-errors)) > - ;; This transform is normally done in the Lisp optimizer, > - ;; so maybe we don't need to bother about it here? > - (setq form (cons 'progn (cdr form))) > - (setq handler #'byte-compile-progn)) > - ((and (or sef (function-get (car form) 'important-return-value)) > - ;; Don't warn for arguments to `ignore'. > - (not (eq byte-compile--for-effect 'for-effect-no-warn)) > - (bytecomp--actually-important-return-value-p form) > - (byte-compile-warning-enabled-p > - 'ignored-return-value (car form))) > - (byte-compile-warn-x > - (car form) > - "value from call to `%s' is unused%s" > - (car form) > - (cond ((eq (car form) 'mapcar) > - "; use `mapc' or `dolist' instead") > - (t ""))))))) > + (and (or sef (function-get (car form) 'important-return-value)) > + ;; Don't warn for arguments to `ignore'. > + (not (eq byte-compile--for-effect 'for-effect-no-warn)) > + (bytecomp--actually-important-return-value-p form) > + (byte-compile-warning-enabled-p > + 'ignored-return-value (car form)) > + (byte-compile-warn-x > + (car form) > + "value from call to `%s' is unused%s" > + (car form) > + (cond ((eq (car form) 'mapcar) > + "; use `mapc' or `dolist' instead") > + (t "")))) > + (and sef (or (eq sef 'error-free) > + byte-compile-delete-errors) > + ;; This transform is normally done in the Lisp optimizer, > + ;; so maybe we don't need to bother about it here? > + (setq form (cons 'progn (cdr form))) > + (setq handler #'byte-compile-progn)))) > > (if (and handler > ;; Make sure that function exists. > > The second class of warnings should not be enabled by default, but maybe > it can become an 'extra warning level which we run once in a while? > > diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el > index 652c79e9c93..4e1e7cd7caf 100644 > --- a/lisp/emacs-lisp/byte-opt.el > +++ b/lisp/emacs-lisp/byte-opt.el > @@ -486,6 +486,15 @@ byte-optimize-form-code-walker > (if-let* ((tmp (byte-opt--fget fn 'side-effect-free))) > (or byte-compile-delete-errors > (eq tmp 'error-free))))) > + (and > + (not (eq for-effect 'for-effect-no-warn)) > + (bytecomp--actually-important-return-value-p form) > + (byte-compile-warning-enabled-p > + 'ignored-return-value (car form)) > + (byte-compile-warn-x > + fn > + "value from call to `%s' is unused" > + fn)) > (byte-compile-log " %s called for effect; deleted" fn) > (byte-optimize-form (cons 'progn (cdr form)) t)) > > But I think to decide that, we need to look at the potential bugs caught > by this code (changed to what I think what was meant): > > diff --git a/lisp/which-key.el b/lisp/which-key.el > index 0be0feb542e..3b339a14bf8 100644 > --- a/lisp/which-key.el > +++ b/lisp/which-key.el > @@ -2464,7 +2464,7 @@ which-key-dump-bindings > (let* ((buffer (get-buffer-create buffer-name)) > (keys (which-key--get-bindings (kbd prefix)))) > (with-current-buffer buffer > - (point-max) > + (goto-char (point-max)) > (save-excursion > (dolist (key keys) > (insert (apply #'format "%s%s%s\n" key))))) > diff --git a/lisp/international/quail.el b/lisp/international/quail.el > index 2b66a0bc0f7..a64a86df91e 100644 > --- a/lisp/international/quail.el > +++ b/lisp/international/quail.el > @@ -2113,7 +2113,7 @@ quail-update-guidance > nil > ;; We want to make both KEY and STR visible, but if the > ;; window is too short, make at least STR visible. > - (setq pos (progn (point) (goto-char pos))) > + (setq pos (prog1 (point) (goto-char pos))) > (beginning-of-line) > (set-window-start win (point)) > (if (not (pos-visible-in-window-p pos win)) > diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el > index 59e1b49f92c..0c358724a39 100644 > --- a/lisp/org/org-table.el > +++ b/lisp/org/org-table.el > @@ -1930,8 +1930,8 @@ org-table-copy-down > (setq field-above > (let ((f (unless (= beg (line-beginning-position)) > (forward-line -1) > - (not (org-at-table-hline-p)) > - (org-table-get-field column)))) > + (and (not (org-at-table-hline-p)) > + (org-table-get-field column))))) > (and (org-string-nw-p f) > (org-trim f))))) > ;; Compute next field. > diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el > index d9044c03aea..5d4137aa7fa 100644 > --- a/lisp/progmodes/ruby-mode.el > +++ b/lisp/progmodes/ruby-mode.el > @@ -1782,8 +1782,8 @@ ruby-forward-sexp > (nth 1 (setq state (apply #'ruby-parse-partial > nil state)))) > (setq expr t) > - (skip-chars-forward "<")) > - (not expr)))) > + (skip-chars-forward "<") > + (not expr))))) > (setq i (1- i))) > ((error) (forward-word-strictly 1))) > i)))) > diff --git a/lisp/progmodes/vhdl-mode.el b/lisp/progmodes/vhdl-mode.el > index 593a83ceffa..0bdb877a10f 100644 > --- a/lisp/progmodes/vhdl-mode.el > +++ b/lisp/progmodes/vhdl-mode.el > @@ -7133,8 +7133,9 @@ vhdl-get-syntactic-context > ((and (save-excursion > (goto-char (1+ containing-sexp)) > (skip-chars-forward " \t") > - (not (eolp)) > - (not (looking-at "--\\|`"))) > + (and > + (not (eolp)) > + (not (looking-at "--\\|`")))) > (save-excursion > (vhdl-beginning-of-statement-1 containing-sexp) > (skip-chars-backward " \t(") > ;; Note: `static-if' can be copied into a package to enable it to be > ;; used in Emacsen older than Emacs 30.1. If the package is used in > > Plus some unnecessary but apparently harmless code: > > diff --git a/lisp/gnus/gnus-score.el b/lisp/gnus/gnus-score.el > index a4102ef0b51..54f07b9d116 100644 > --- a/lisp/gnus/gnus-score.el > +++ b/lisp/gnus/gnus-score.el > @@ -1489,7 +1489,6 @@ gnus-score-save > (setq gnus-score-alist nil) > (nnheader-set-temp-buffer " *Gnus Scores*") > (while cache > - (current-buffer) > (setq entry (pop cache) > file (nnheader-translate-file-chars (car entry) t) > score (cdr entry)) > diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el > index 74dff3217ff..7e27019bfb4 100644 > --- a/lisp/progmodes/gdb-mi.el > +++ b/lisp/progmodes/gdb-mi.el > @@ -1643,7 +1643,6 @@ gdb-buffer-type > (defun gdb-buffer-shows-main-thread-p () > "Return t if current GDB buffer shows main selected thread and > is not bound to it." > - (current-buffer) > (not (local-variable-p 'gdb-thread-number))) > > (defun gdb-get-buffer (buffer-type &optional thread) > diff --git a/lisp/textmodes/table.el b/lisp/textmodes/table.el > index 0e024f9261e..3e6954256a6 100644 > --- a/lisp/textmodes/table.el > +++ b/lisp/textmodes/table.el > @@ -5360,10 +5360,8 @@ table--goto-coordinate > (if (/= (move-to-column x) x) > (if (> (move-to-column x) x) > (if no-tab-expansion > - (progn > - (while (> (move-to-column x) x) > - (setq x (1- x))) > - (point)) > + (while (> (move-to-column x) x) > + (setq x (1- x))) > (throw 'exit (move-to-column x t))) > (throw 'exit nil))) > (move-to-column x t)) > diff --git a/lisp/speedbar.el b/lisp/speedbar.el > index b1fd141321c..2b54c456651 100644 > --- a/lisp/speedbar.el > +++ b/lisp/speedbar.el > @@ -1232,8 +1232,7 @@ speedbar-mode > dframe-mouse-click-function #'speedbar-click > dframe-mouse-position-function #'speedbar-position-cursor-on-line) > (setq-local tab-bar-mode nil) > - (setq tab-line-exclude nil)) > - speedbar-buffer) > + (setq tab-line-exclude nil))) > > (define-obsolete-function-alias 'speedbar-message 'dframe-message "24.4") > > > So five potentially user-visible bugs in the current Emacs code base, > plenty of false positives for the second change, at least. Stefan and Mattias, any comments or suggestions?
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sat, 28 Jun 2025 17:40:02 GMT) Full text and rfc822 format available.Message #11 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Pip Cet <pipcet <at> protonmail.com> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 28 Jun 2025 13:38:56 -0400
> We do not currently warn consistently when discarding the value of a > function call to a side-effect-free function, even though the return > value is usually important. It's not clear that we should, but I tried > enabling such warnings and found ~5 questionable places in the code in > lisp/. > 1. We don't ever warn about side-effect-and-error-free functions like > "not"; this seems to me to be strange behavior on the part of the byte > compiler, but it's probably been that way for a long time. There are several macros which rely on the byte-compiler silently erasing such "side-effect-and-error-free" if it's not used. There is a warning for those that can signal errors *because* we can't safely erase them. > For example, this "not" in an implicit progn, in vc/vc.el: > > (insert-string (format "Changes to %s since last lock:\n\n" > file)) > (not (beep)) > (yes-or-no-p > (concat "File has unlocked changes, " > "claim lock retaining changes? "))) > > That's commit bbf97570e5037b2417c9dd6353e1b2a9afadda7c, from 1993, moved > around in 2000 but essentially untouched. Has the byte compiler been > silent on this situation for all this time? Probably. > Is it worth changing? Often, it's hard for macros to know if a value will be used or not, so if we want to change it, we need a convenient way for macros to let the compiler erase those operations silently. It's not completely trivial because we can't just wrap them like (FOO (not (beep))) since the annotation should apply to the `not` operation but not to its argument (the `beep` operation). > 2. We also don't warn about function calls which we optimize away; this > also seems problematic if the form we optimize away was present in the > original source code, but it's easier to write macros which generate > intermediate forms which can then be optimized, and I don't think we can > currently tell whether we're looking at macro-expanded code or not. Exactly. > For example, my naive experiment produced false warnings when > > (pop list2) > > appeared in a context in which its return value was ignored, which is a > common idiom and probably accounts for the majority of the warnings > produced. Similarly, `pcase` destructuring was changed to use `c[ad]r-safe` (even when we just did a `consp` check) rather than `c[ad]r` to avoid such warnings. > (if (not m) > ;; Go to dependencies, and search there. > nil) > m)))) > > probably should generate a warning, because the if form seems completely > mysterious to me. At the same time: is it an actual problem? Other than clean up the code, what would be the benefit? The above code shape is the kind of thing that can easily be produced by macros, so again we'd need some way for macros to avoid/silence the warnings without too much effort. I'm not sure it's worth the trouble here. > My suggestion is to fix the first issue and complain about such forms, > by doing something like this: > > diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el > index 5fa65ff71a6..4716e150faf 100644 > --- a/lisp/emacs-lisp/bytecomp.el > +++ b/lisp/emacs-lisp/bytecomp.el > @@ -3470,26 +3470,25 @@ byte-compile-form > > (when byte-compile--for-effect > (let ((sef (function-get (car form) 'side-effect-free))) > - (cond > - ((and sef (or (eq sef 'error-free) > - byte-compile-delete-errors)) > - ;; This transform is normally done in the Lisp optimizer, > - ;; so maybe we don't need to bother about it here? > - (setq form (cons 'progn (cdr form))) > - (setq handler #'byte-compile-progn)) > - ((and (or sef (function-get (car form) 'important-return-value)) > - ;; Don't warn for arguments to `ignore'. > - (not (eq byte-compile--for-effect 'for-effect-no-warn)) > - (bytecomp--actually-important-return-value-p form) > - (byte-compile-warning-enabled-p > - 'ignored-return-value (car form))) > - (byte-compile-warn-x > - (car form) > - "value from call to `%s' is unused%s" > - (car form) > - (cond ((eq (car form) 'mapcar) > - "; use `mapc' or `dolist' instead") > - (t ""))))))) > + (and (or sef (function-get (car form) 'important-return-value)) > + ;; Don't warn for arguments to `ignore'. > + (not (eq byte-compile--for-effect 'for-effect-no-warn)) > + (bytecomp--actually-important-return-value-p form) > + (byte-compile-warning-enabled-p > + 'ignored-return-value (car form)) > + (byte-compile-warn-x > + (car form) > + "value from call to `%s' is unused%s" > + (car form) > + (cond ((eq (car form) 'mapcar) > + "; use `mapc' or `dolist' instead") > + (t "")))) > + (and sef (or (eq sef 'error-free) > + byte-compile-delete-errors) > + ;; This transform is normally done in the Lisp optimizer, > + ;; so maybe we don't need to bother about it here? > + (setq form (cons 'progn (cdr form))) > + (setq handler #'byte-compile-progn)))) IIUC you're swapping the two tests such that an `important-return-value` is not missed just because we optimized it away according to its `error-free` annotation? I can go along with that. > The second class of warnings should not be enabled by default, but maybe > it can become an 'extra warning level which we run once in a while? Adding a notion of "level" for warnings that aren't super-important might make sense, yes. I'd still welcome some way to silence those warnings locally rather than via some global setting. Stefan
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sat, 28 Jun 2025 21:56:03 GMT) Full text and rfc822 format available.Message #14 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Pip Cet <pipcet <at> protonmail.com> To: Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 28 Jun 2025 21:55:33 +0000
"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes: >> We do not currently warn consistently when discarding the value of a >> function call to a side-effect-free function, even though the return >> value is usually important. It's not clear that we should, but I tried >> enabling such warnings and found ~5 questionable places in the code in >> lisp/. > >> 1. We don't ever warn about side-effect-and-error-free functions like >> "not"; this seems to me to be strange behavior on the part of the byte >> compiler, but it's probably been that way for a long time. > > There are several macros which rely on the byte-compiler silently > erasing such "side-effect-and-error-free" if it's not used. I understand that the macro case is important and should be solved; there are many ways to do so, such as introducing a maybe-ignore function and using it in macros (analogously to ignore suppressing for-effect warnings). (I think we could also keep track of which cons cells were in the original program and which ones arose through macroexpansion, but this failed to work right away...). So let's ignore that problem for now, and decide whether a warning is desirable assuming it can be suppressed for certain (or, even better, all) macros. > There is a warning for those that can signal errors *because* we can't > safely erase them. (Erasing the other ones hides bugs, so it's "safe" only for correct code). And there are several places where we have to work around the warning we have, because we're intentionally evaluating a side-effect-free function for effect (to see if they throw). Introducing 'maybe-ignore' in a few select places doesn't seem to me to be too much of a problem. >> For example, this "not" in an implicit progn, in vc/vc.el: >> >> (insert-string (format "Changes to %s since last lock:\n\n" >> file)) >> (not (beep)) >> (yes-or-no-p >> (concat "File has unlocked changes, " >> "claim lock retaining changes? "))) >> >> That's commit bbf97570e5037b2417c9dd6353e1b2a9afadda7c, from 1993, moved >> around in 2000 but essentially untouched. Has the byte compiler been >> silent on this situation for all this time? > > Probably. >> Is it worth changing? > > Often, it's hard for macros to know if a value will be used or not, so > if we want to change it, we need a convenient way for macros to let the > compiler erase those operations silently. I suspect we'd need two ways: one which suppresses the warning recursively (lexical scope, of course, not dynamic), for all sub^n-arguments, and one which suppresses it only for one function call. My suggestion would be to use with-suppressed-warnings for the first case, and 'maybe-ignore' for the second case, and to apply this: diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 7c5991cb633..ad447656e38 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -380,7 +380,8 @@ byte-compile-warning-enabled-p (let ((suppress nil)) (dolist (elem byte-compile--suppressed-warnings) (when (and (eq (car elem) warning) - (memq symbol (cdr elem))) + (or (eq (cdr elem) t) + (memq symbol (cdr elem)))) (setq suppress t))) (and (not suppress) ;; During an Emacs build, we want all warnings. to allow (with-suppressed-warnings ((ignored-return-value . t)) BODY) > It's not completely trivial because we can't just wrap them like (FOO > (not (beep))) since the annotation should apply to the `not` operation > but not to its argument (the `beep` operation). I'm not sure I understand; the beep operation has no parameters, and wouldn't ignore its parameter if it had one. IIUC, you're saying that we need the non-recursive option above, but not the recursive one? I'm not sure I agree, but would that really be hard? We already have a recursive option which works, and the non-recursive option would be just like 'ignore'. >> (if (not m) >> ;; Go to dependencies, and search there. >> nil) >> m)))) >> >> probably should generate a warning, because the if form seems completely >> mysterious to me. > > At the same time: is it an actual problem? Mysterious code which no one can now determine the intention of? Sounds like a problem to me! > Other than clean up the code, what would be the benefit? Other than cleaning up existing code, and much more importantly, it'd help people write new code :-) > I'm not sure it's worth the trouble here. I'm not sure either; TBH, I wrote this mostly in case there was an unmanageable third category of cases where we don't want the warnings to appear, in addition to macro expansions (which I think are solvable) and very special corner cases like benchmarks. So far, there doesn't appear to be, so maybe this is worth thinking about more. >> My suggestion is to fix the first issue and complain about such forms, >> by doing something like this: >> >> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el >> index 5fa65ff71a6..4716e150faf 100644 >> --- a/lisp/emacs-lisp/bytecomp.el >> +++ b/lisp/emacs-lisp/bytecomp.el >> @@ -3470,26 +3470,25 @@ byte-compile-form >> >> (when byte-compile--for-effect >> (let ((sef (function-get (car form) 'side-effect-free))) >> - (cond >> - ((and sef (or (eq sef 'error-free) >> - byte-compile-delete-errors)) >> - ;; This transform is normally done in the Lisp optimizer, >> - ;; so maybe we don't need to bother about it here? >> - (setq form (cons 'progn (cdr form))) >> - (setq handler #'byte-compile-progn)) >> - ((and (or sef (function-get (car form) 'important-return-value)) >> - ;; Don't warn for arguments to `ignore'. >> - (not (eq byte-compile--for-effect 'for-effect-no-warn)) >> - (bytecomp--actually-important-return-value-p form) >> - (byte-compile-warning-enabled-p >> - 'ignored-return-value (car form))) >> - (byte-compile-warn-x >> - (car form) >> - "value from call to `%s' is unused%s" >> - (car form) >> - (cond ((eq (car form) 'mapcar) >> - "; use `mapc' or `dolist' instead") >> - (t ""))))))) >> + (and (or sef (function-get (car form) 'important-return-value)) >> + ;; Don't warn for arguments to `ignore'. >> + (not (eq byte-compile--for-effect 'for-effect-no-warn)) >> + (bytecomp--actually-important-return-value-p form) >> + (byte-compile-warning-enabled-p >> + 'ignored-return-value (car form)) >> + (byte-compile-warn-x >> + (car form) >> + "value from call to `%s' is unused%s" >> + (car form) >> + (cond ((eq (car form) 'mapcar) >> + "; use `mapc' or `dolist' instead") >> + (t "")))) >> + (and sef (or (eq sef 'error-free) >> + byte-compile-delete-errors) >> + ;; This transform is normally done in the Lisp optimizer, >> + ;; so maybe we don't need to bother about it here? >> + (setq form (cons 'progn (cdr form))) >> + (setq handler #'byte-compile-progn)))) > > IIUC you're swapping the two tests such that an `important-return-value` > is not missed just because we optimized it away according to its > `error-free` annotation? Basically, yes. We still perform the deletion in what becomes a fall-through case, but as the comment says, either the byte optimizer was disabled and we probably shouldn't optimize the code (but still complain!), or the byte optimizer missed an optimization and we should complain about that instead. > I can go along with that. >> The second class of warnings should not be enabled by default, but maybe >> it can become an 'extra warning level which we run once in a while? > > Adding a notion of "level" for warnings that aren't super-important > might make sense, yes. OTOH, wasn't that what elint was for? > I'd still welcome some way to silence those > warnings locally rather than via some global setting. (with-suppressed-warnings ((lexical . t)) ... various incomplete defuns ... ) is certainly something I'm going to use :-) Pip
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sat, 28 Jun 2025 22:27:02 GMT) Full text and rfc822 format available.Message #17 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Pip Cet <pipcet <at> protonmail.com> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 28 Jun 2025 18:26:06 -0400
> I suspect we'd need two ways: one which suppresses the warning > recursively (lexical scope, of course, not dynamic), for all > sub^n-arguments, and one which suppresses it only for one function call. Could be. The "all sub arguments" case is easy to fix (but I'm not sure we'd ever need/want it; examples welcome), it's the other one that's more interesting. >> It's not completely trivial because we can't just wrap them like (FOO >> (not (beep))) since the annotation should apply to the `not` operation >> but not to its argument (the `beep` operation). > I'm not sure I understand; the beep operation has no parameters, and > wouldn't ignore its parameter if it had one. The `beep` operation is the parameter to `not`. > IIUC, you're saying that we need the non-recursive option above, but not > the recursive one? That's right. > I'm not sure I agree, Why? We do want to `beep` here. The only part that we should remove is the silly `not` operation. > but would that really be hard? Don't know. > We already have a recursive option which works, and the non-recursive > option would be just like 'ignore'. Maybe you're right, I simply don't know. >>> (if (not m) >>> ;; Go to dependencies, and search there. >>> nil) >>> m)))) >>> >>> probably should generate a warning, because the if form seems completely >>> mysterious to me. >> At the same time: is it an actual problem? > Mysterious code which no one can now determine the intention of? > Sounds like a problem to me! We have an endless supply of such code, tho, most of which doesn't have the property of being "cosmetically silly". >>> The second class of warnings should not be enabled by default, but maybe >>> it can become an 'extra warning level which we run once in a while? >> Adding a notion of "level" for warnings that aren't super-important >> might make sense, yes. > OTOH, wasn't that what elint was for? Could be. I don't know much about elint, other than that it never seemed to do anything very useful when I tried it. In any case, whether it's implement in the compiler or in a separate package, the requirement remains to provide a way to silence the false-positives. >> I'd still welcome some way to silence those >> warnings locally rather than via some global setting. > > (with-suppressed-warnings ((lexical . t)) > ... various incomplete defuns ... > ) But if you use it within a macro around code provided by the caller, you risk silencing real problems. So, for the same reason we moved from `with-no-warnings` to `with-suppressed-warnings`, I'd rather have something that has a more local effect. This said, maybe we want a general "single-level" version of `with-suppressed-warnings` since I've already several times wanted a way to suppress a particular kind of warning but only for the top-level of an expression and not for all the sub-expressions it may hold. I think such a ". t" option would be good for a "single-level" warning suppressor since in most cases it's obvious which warning it would apply to. For `with-suppressed-warnings`, OTOH, I'm not sure I want to encourage people to suppress all lexical warnings within all the subexpressions of a form. Stefan
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Sun, 29 Jun 2025 18:26:02 GMT) Full text and rfc822 format available.Message #20 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Pip Cet <pipcet <at> protonmail.com> To: Stefan Monnier <monnier <at> iro.umontreal.ca> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sun, 29 Jun 2025 18:25:02 +0000
"Stefan Monnier" <monnier <at> iro.umontreal.ca> writes: >> I suspect we'd need two ways: one which suppresses the warning >> recursively (lexical scope, of course, not dynamic), for all >> sub^n-arguments, and one which suppresses it only for one function call. > > Could be. The "all sub arguments" case is easy to fix (but I'm not > sure we'd ever need/want it; examples welcome), it's the other one > that's more interesting. I was thinking about pcase, cl-loop, and the cconv code; I expected all three to generate huge and complicated expressions which it might not be feasible to mix (maybe-ignore ...) calls into. >>> It's not completely trivial because we can't just wrap them like (FOO >>> (not (beep))) since the annotation should apply to the `not` operation >>> but not to its argument (the `beep` operation). >> I'm not sure I understand; the beep operation has no parameters, and >> wouldn't ignore its parameter if it had one. > > The `beep` operation is the parameter to `not`. But it doesn't generate a warning, so how does the annotation apply to it? >> IIUC, you're saying that we need the non-recursive option above, but not >> the recursive one? > > That's right. Definining a function called maybe-ignore, which returns its argument but has no compiler macro, seems to work. >> I'm not sure I agree, > > Why? We do want to `beep` here. The only part that we should remove is > the silly `not` operation. I wasn't talking about removing the expression recursively (which I agree doesn't make much sense), just about disabling the warnings in an entire lexical scope. >> We already have a recursive option which works, and the non-recursive >> option would be just like 'ignore'. > > Maybe you're right, I simply don't know. I'll test this some more. Rerunning this on the Emacs tree, I think things like the which-key.el bug or the ob-tangle.el one are worth fixing. (However, I'm not quite sure why the ob-tangle.el code triggered the warning :-) ) I'm not sure they were in the first patch; here they are for completeness: diff --git a/lisp/which-key.el b/lisp/which-key.el index 0be0feb542e..3b339a14bf8 100644 --- a/lisp/which-key.el +++ b/lisp/which-key.el @@ -2464,7 +2464,7 @@ which-key-dump-bindings (let* ((buffer (get-buffer-create buffer-name)) (keys (which-key--get-bindings (kbd prefix)))) (with-current-buffer buffer - (point-max) + (goto-char (point-max)) (save-excursion (dolist (key keys) (insert (apply #'format "%s%s%s\n" key))))) diff --git a/lisp/org/ob-tangle.el b/lisp/org/ob-tangle.el index 75e7b560848..e3c1d7a45b4 100644 --- a/lisp/org/ob-tangle.el +++ b/lisp/org/ob-tangle.el @@ -296,7 +296,8 @@ org-babel-tangle (when she-bang (unless tangle-mode (setq tangle-mode #o755))) (when tangle-mode - (add-to-list 'modes (org-babel-interpret-file-mode tangle-mode))) + (cl-pushnew (org-babel-interpret-file-mode tangle-mode) + modes)) ;; Possibly create the parent directories for file. (let ((m (funcall get-spec :mkdirp))) (and m fnd (not (string= m "no")) (Tangent: we failed to throw an error for that last one because we were in a lambda which captured modes; would this be a good idea, maybe?) diff --git a/lisp/subr.el b/lisp/subr.el index 69f6e4dbab8..4ab1a1b84b7 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -2523,7 +2523,7 @@ add-to-list (warnfun (lambda () ;; FIXME: We should also emit a warning for let-bound ;; variables with dynamic binding. - (when (assq sym byte-compile--lexical-environment) + (when (memq sym byte-compile-lexical-variables) (byte-compile-report-error msg :fill)))) (code (macroexp-let2 macroexp-copyable-p x element > >>>> (if (not m) >>>> ;; Go to dependencies, and search there. >>>> nil) >>>> m)))) >>>> >>>> probably should generate a warning, because the if form seems completely >>>> mysterious to me. >>> At the same time: is it an actual problem? >> Mysterious code which no one can now determine the intention of? >> Sounds like a problem to me! > > We have an endless supply of such code, tho, most of which doesn't have > the property of being "cosmetically silly". I don't think all the bugs were cosmetic; the (not (beep)) one certainly is, though. >>>> The second class of warnings should not be enabled by default, but maybe >>>> it can become an 'extra warning level which we run once in a while? >>> Adding a notion of "level" for warnings that aren't super-important >>> might make sense, yes. >> OTOH, wasn't that what elint was for? > > Could be. I don't know much about elint, other than that it never > seemed to do anything very useful when I tried it. > > In any case, whether it's implement in the compiler or in a separate > package, the requirement remains to provide a way to silence the > false-positives. Absolutely. In my current test runs, 'push' appears to be the only false positive, and requires a 'maybe-ignore'. >>> I'd still welcome some way to silence those >>> warnings locally rather than via some global setting. >> >> (with-suppressed-warnings ((lexical . t)) >> ... various incomplete defuns ... >> ) > > But if you use it within a macro around code provided by the caller, you > risk silencing real problems. Yes, that's always a risk when trying to reduce the number of false positives. > So, for the same reason we moved from `with-no-warnings` to > `with-suppressed-warnings`, I'd rather have something that has a more > local effect. I'll have to look at pcase.el to see whether that's feasible. > This said, maybe we want a general "single-level" version of > `with-suppressed-warnings` since I've already several times wanted a way > to suppress a particular kind of warning but only for the top-level of > an expression and not for all the sub-expressions it may hold. That sounds like a good idea. > I think such a ". t" option would be good for a "single-level" > warning suppressor since in most cases it's obvious which warning it > would apply to. > For `with-suppressed-warnings`, OTOH, I'm not sure I want to encourage > people to suppress all lexical warnings within all the subexpressions of > a form. I agree, but the problem isn't the . t, it's the very broad 'lexical category. Only the "unused" messages should really be disabled. Maybe it's time to split it up. Pip
bug-gnu-emacs <at> gnu.org
:bug#78716
; Package emacs
.
(Mon, 30 Jun 2025 19:34:02 GMT) Full text and rfc822 format available.Message #23 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Pip Cet <pipcet <at> protonmail.com> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Mon, 30 Jun 2025 15:33:37 -0400
>>> I suspect we'd need two ways: one which suppresses the warning >>> recursively (lexical scope, of course, not dynamic), for all >>> sub^n-arguments, and one which suppresses it only for one function call. >> Could be. The "all sub arguments" case is easy to fix (but I'm not >> sure we'd ever need/want it; examples welcome), it's the other one >> that's more interesting. > I was thinking about pcase, cl-loop, and the cconv code; I expected all > three to generate huge and complicated expressions which it might not be > feasible to mix (maybe-ignore ...) calls into. But if you silence all the warnings in the subtree, then it will also silence unrelated warnings in the branches of the `pcase` which we don't want. >>>> It's not completely trivial because we can't just wrap them like (FOO >>>> (not (beep))) since the annotation should apply to the `not` operation >>>> but not to its argument (the `beep` operation). >>> I'm not sure I understand; the beep operation has no parameters, and >>> wouldn't ignore its parameter if it had one. >> The `beep` operation is the parameter to `not`. > But it doesn't generate a warning, so how does the annotation apply to > it? When the argument is literally `(beep)` you're right, but I used it only to indicate the position of the thing to which it could incorrectly apply. Think of `(not (progn (< 0 x) (< x 10)))` Stefan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.