From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.bugs Subject: bug#68938: Emacs "master". Incorrect code generated by pcase. Date: Tue, 6 Feb 2024 12:58:53 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23909"; mail-complaints-to="usenet@ciao.gmane.io" Cc: acm@muc.de, 68938@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Feb 06 14:00:01 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rXL3Q-00067Q-VF for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 06 Feb 2024 14:00:01 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rXL3H-0007Bw-9c; Tue, 06 Feb 2024 07:59:51 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rXL3F-0007Bg-Gk for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 07:59:49 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rXL3F-0001U4-91 for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 07:59:49 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rXL3S-00076X-CI for bug-gnu-emacs@gnu.org; Tue, 06 Feb 2024 08:00:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Mackenzie Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 06 Feb 2024 13:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68938 X-GNU-PR-Package: emacs Original-Received: via spool by 68938-submit@debbugs.gnu.org id=B68938.170722435627207 (code B ref 68938); Tue, 06 Feb 2024 13:00:02 +0000 Original-Received: (at 68938) by debbugs.gnu.org; 6 Feb 2024 12:59:16 +0000 Original-Received: from localhost ([127.0.0.1]:53391 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rXL2i-00074k-9S for submit@debbugs.gnu.org; Tue, 06 Feb 2024 07:59:16 -0500 Original-Received: from mail.muc.de ([193.149.48.3]:24224) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rXL2f-00074P-VY for 68938@debbugs.gnu.org; Tue, 06 Feb 2024 07:59:14 -0500 Original-Received: (qmail 31812 invoked by uid 3782); 6 Feb 2024 13:58:54 +0100 Original-Received: from acm.muc.de (p4fe15bf0.dip0.t-ipconnect.de [79.225.91.240]) (using STARTTLS) by colin.muc.de (tmda-ofmipd) with ESMTP; Tue, 06 Feb 2024 13:58:53 +0100 Original-Received: (qmail 18446 invoked by uid 1000); 6 Feb 2024 12:58:53 -0000 Content-Disposition: inline In-Reply-To: X-Submission-Agent: TMDA/1.3.x (Ph3nix) X-Primary-Address: acm@muc.de X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:279491 Archived-At: Hello, Stefan. On Mon, Feb 05, 2024 at 13:27:21 -0500, Stefan Monnier wrote: > > In a development version of Emacs, last synched with master in December, > > I have added the following pcase clause to macroexp--expand-all in > > lisp/emacs-lisp/macroexp.el: > > (`(,(and 'defalias d > > (guard (and (null defining-symbol) > > (symbol-with-pos-p d)))) > > ',sym . ,_) > > ;; Here, don't change the form; just set `defining-symbol' > > ;; for a (defalias 'foo ...) in the source code. > > ;; (when (symbol-with-pos-p d) > > (setq defining-symbol sym) > > form) > Where did do you add it? Just before the (`(function ,(and f `(lambda . ,_))) clause, like you did. > > .. pcase expands that clause to this cond clause: > > ((eq x0 'defalias) > > (cond > > ((let* ((d x0)) > > (and (null defining-symbol) (symbol-with-pos-p d))) > > (let* ((x14 (cdr-safe form))) > > (cond > > ((consp x14) > > (let* ((x15 (car-safe x14))) > > (cond > > ((consp x15) > > (let* ((x16 (car-safe x15))) > > (cond > > ((eq x16 'quote) > > (let* ((x17 (cdr-safe x15))) > > (cond > > ((consp x17) > > (let* ((x18 (car-safe x17)) (x19 (cdr-safe x17))) > > (cond > > ((null x19) > > (let ((d x0) (sym x18)) > > (ignore d) (setq defining-symbol sym) form)) > > ((consp x0) > > (let* ((x21 (car-safe x0))) > > (if (eq x21 'lambda) (funcall pcase-3 x0 x14) > > (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0))))) > > ((consp x0) > > (let* ((x23 (car-safe x0))) > > (if (eq x23 'lambda) (funcall pcase-3 x0 x14) > > (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0))))) > > ((consp x0) > > (let* ((x25 (car-safe x0))) > > (if (eq x25 'lambda) (funcall pcase-3 x0 x14) > > (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0))))) > > ((consp x0) > > (let* ((x27 (car-safe x0))) > > (if (eq x27 'lambda) (funcall pcase-3 x0 x14) > > (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0))))) > > ((consp x0) > > (let* ((x29 (car-safe x0))) > > (if (eq x29 'lambda) (funcall pcase-3 x0 x14) (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0))))) > > ((consp x0) > > (let* ((x31 (car-safe x0))) > > (if (eq x31 'lambda) > > (let* ((x33 (cdr-safe form))) (funcall pcase-3 x0 x33)) > > (funcall pcase-2 x0)))) > > (t (funcall pcase-2 x0)))) > > > > .. This contains errors: > > > > (i) Although it has been established that x0 is 'defalias, there are many > > tests (consp x0). > Yup, clearly some missed optimization. > I added your code just before the > (`(function ,(and f `(lambda . ,_))) > branch, and I didn't see such poor code, so it seems that it depends on > further details. Yes. I think my bug report was premature at best. I was part way through amending backquote.el to be able to add the position information into things like the lambda in (push `(,bsym (lambda ,(mapcar #'car varvals) ,@ignores ,@code)) defs)) in pcase--expand. I think this bit of code deals with creating pcase-1, etc., so it seems highly likely my tentative changes were to blame. > On further inspection I see a similar problem in another branch, > where it generated: > ((pcase--flip memq '(defconst defvar) x71) > (let* ((x78 (cdr-safe form))) > (cond > ((consp x78) > (let* ((x79 (car-safe x78))) > (cond > ((symbolp x79) > (let ((name x79)) > (push name macroexp--dynvars) > (macroexp--all-forms form 2))) > ((consp x71) > (let* ((x81 (car-safe x71))) > (if (eq x81 'lambda) (funcall pcase-2 x71 x78) > (funcall pcase-1 x71)))) > (t (funcall pcase-1 x71))))) > ((consp x71) > (let* ((x83 (car-safe x71))) > (if (eq x83 'lambda) (funcall pcase-2 x71 x78) > (funcall pcase-1 x71)))) > (t (funcall pcase-1 x71))))) > where we do that same useless (consp x71) test. I think this case is > "normal" (the branch's test is basically (memq x71 '(defconst defvar), > i.e. more complex than (eq x71 'defalias)) and I seem to remember > consciously punting on handling such things in > `pcase--mutually-exclusive-p`. > Your case doesn't sound like one I'm aware of, OTOH. I think I should just close the bug as not a bug. It was one of these things I couldn't understand or do anything about, but right after reporting it, the solution became obvious, and I couldn't reproduce the bug easily any more. I've got my amendments to backquote.el working, now. > > (ii) There are calls of the form (funcall pcase-2 x0), i.e. (funcall > > pcase-2 'defalias). This causes a wrong-number-of-arguments error. > I don't see this problem here. In my case it's > (funcall pcase-1 x71) > but the number of arguments is right since pcase-1 is defined a bit > earlier as: > (lambda (func) > (let ((handler (function-get func 'compiler-macro))) > (if (null handler) (macroexp--all-forms form 1) > (unless (functionp handler) > (with-demoted-errors "macroexp--expand-all: %S" > (autoload-do-load (indirect-function func) func))) > (let ((newform (macroexp--compiler-macro handler form))) > (if (eq form newform) > (if > (equal form > (setq newform > (macroexp--all-forms form 1))) > form > (setq form > (macroexp--compiler-macro handler newform)) > (if (eq newform form) newform > (macroexp--expand-all form))) > (macroexp--expand-all newform)))))) > > (iii) There is no sign of the final `form' being returned, though this > > may be being done elsewhere. > I see the following in your code sample: > (cond > ((null x19) > (let ((d x0) (sym x18)) > (ignore d) (setq defining-symbol sym) form)) > which seems to be correctly returning `form`. Yes. Sorry about this bug report, which turned out to be a time waster. > Stefan -- Alan Mackenzie (Nuremberg, Germany).