From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#68938: Emacs "master". Incorrect code generated by pcase. Date: Mon, 05 Feb 2024 13:27:21 -0500 Message-ID: References: Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31859"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 68938@debbugs.gnu.org To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Feb 05 19:28:05 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 1rX3hN-0007zB-3n for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 05 Feb 2024 19:28:05 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rX3hB-0002ix-79; Mon, 05 Feb 2024 13:27:53 -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 1rX3h9-0002il-I3 for bug-gnu-emacs@gnu.org; Mon, 05 Feb 2024 13:27:51 -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 1rX3h9-0000le-6M for bug-gnu-emacs@gnu.org; Mon, 05 Feb 2024 13:27:51 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rX3hK-0001mN-Fb for bug-gnu-emacs@gnu.org; Mon, 05 Feb 2024 13:28:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 05 Feb 2024 18:28: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.17071576676817 (code B ref 68938); Mon, 05 Feb 2024 18:28:02 +0000 Original-Received: (at 68938) by debbugs.gnu.org; 5 Feb 2024 18:27:47 +0000 Original-Received: from localhost ([127.0.0.1]:52275 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rX3h5-0001lt-A1 for submit@debbugs.gnu.org; Mon, 05 Feb 2024 13:27:47 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:23279) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rX3h1-0001ld-Tj for 68938@debbugs.gnu.org; Mon, 05 Feb 2024 13:27:45 -0500 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 83B5E100390; Mon, 5 Feb 2024 13:27:24 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1707157643; bh=hSTZVlyTI33KjXqwstWvn/WoaMY0VOZ6HxQ+5mKHRNg=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=ImSdSGhmPGekaQTlvAfyYJgWDRLKKl6Ja6ZvhEUU1/nRZMHJKVMLaRDxPrQAEUuTI y/xpahJAwizpgNJ/NGUsmmVo+pJFX42xNToazojnKRp9loPLwIsQc+c5eiQmJXhp6U CaJa7H5WuLcp0oduN26cz+I1r8ck5amA1cOxaO1m+yFVAqc1rXbavicYO7KVBnKM4H L1YYOGYfsaH3mXewa9Jc+ITZg9aU6WsSpKJ5YCBULA3d0ysILcwiMtey+mIrkhb/vb 96Oxy3ui3e3Q1Lus4KOdO6jUBut2wm4Ok2MfbuBvwGI9XoDN6qSdjzcbUgtOTXlV9X hH5BoNZdyV95g== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 6472A1002B0; Mon, 5 Feb 2024 13:27:23 -0500 (EST) Original-Received: from pastel (69-165-153-17.dsl.teksavvy.com [69.165.153.17]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 042031207A7; Mon, 5 Feb 2024 13:27:22 -0500 (EST) In-Reply-To: (Alan Mackenzie's message of "Mon, 5 Feb 2024 17:18:40 +0000") 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:279462 Archived-At: > 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? > .. 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. 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. > (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`. Stefan