From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#43100: 28.0.50; pcase not binding variables conditionally Date: Sun, 30 Aug 2020 14:07:47 -0400 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="29219"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Philipp Stephani , 43100@debbugs.gnu.org To: Pip Cet Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Aug 30 20:08:09 2020 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 1kCRkO-0007Vl-G6 for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 30 Aug 2020 20:08:09 +0200 Original-Received: from localhost ([::1]:53350 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kCRkN-00025n-8I for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 30 Aug 2020 14:08:07 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53304) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kCRkI-00025f-Jp for bug-gnu-emacs@gnu.org; Sun, 30 Aug 2020 14:08:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:39166) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kCRkI-0005QU-BD for bug-gnu-emacs@gnu.org; Sun, 30 Aug 2020 14:08:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kCRkI-00013H-5e for bug-gnu-emacs@gnu.org; Sun, 30 Aug 2020 14:08:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 30 Aug 2020 18:08:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 43100 X-GNU-PR-Package: emacs Original-Received: via spool by 43100-submit@debbugs.gnu.org id=B43100.15988108794035 (code B ref 43100); Sun, 30 Aug 2020 18:08:02 +0000 Original-Received: (at 43100) by debbugs.gnu.org; 30 Aug 2020 18:07:59 +0000 Original-Received: from localhost ([127.0.0.1]:50712 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kCRkF-000131-FM for submit@debbugs.gnu.org; Sun, 30 Aug 2020 14:07:59 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:52442) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kCRkD-00012m-2v for 43100@debbugs.gnu.org; Sun, 30 Aug 2020 14:07:58 -0400 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 0EE8080B69; Sun, 30 Aug 2020 14:07:51 -0400 (EDT) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id D69498066B; Sun, 30 Aug 2020 14:07:48 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1598810868; bh=dVo02K/GJSZKK0+eG6fgaUVb47Yhg/ndzsvRMUils0I=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=cemW9EOLMRQ8a54JT8aMV0UgvhUgOD3pJnHthJopMqoTL5bz/YRHQm5UszRfoxvou jZukgbvQ6CgXd/lQiuRm6c7Vrq9xNgnFq71a/1+Q0Go75omat2mMoEFvChmURPul0A lIrLWw0ljsUQ/cTfHuKQr2pDlnGVmd/mJMEFTe8v88oTv1r9QQgw2JJMoXwtGg+vS3 IaorihiIdKmQT1U0uRv7cK+uB3I5E9r5wWw4akqcAO24MME5u9Yi1a5b/g550sZd4j P+VfnBhJlwFl0pPdAqM+xJBgNDqojE0aK5UUZaIXJlWaL0ge58eYHEAyvHWLDySfRE O40OW2LWtX+kg== Original-Received: from alfajor (unknown [45.72.232.131]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id A3BF4120535; Sun, 30 Aug 2020 14:07:48 -0400 (EDT) In-Reply-To: (Pip Cet's message of "Sun, 30 Aug 2020 16:21:42 +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" Xref: news.gmane.io gmane.emacs.bugs:186718 Archived-At: >> IIUC you want >> >> (pcase V >> ((or (pred symbolp) name) >> (let ((foo 'bar)) name))) >> >> to behave like >> >> (cond >> ((symbolp V) (let ((foo 'bar)) name)) >> (t (let ((name V)) (let ((foo 'bar)) name)))) >> >> ? > > Yes, that's correct. It's also how (pcase V ((or (pred symbolp) name) > name) behaves... Indeed, but that's an accident. Ideally it should either signal an error at macro-expansion time, or return nil when V is a symbol. Since the current implementation doesn't go to the effort of doing either of those, we instead limit ourselves to recommend against using such patterns (IOW, "use at your own risks"). >> I'd rather not go there > You'd rather have the behavior of (pcase V ((or pred symbolp) name) > EXPR) depend on the complexity of EXPR? More specifically, I'd rather not choose a semantics that imposes duplicating the branch body, since we have no control over its size and that can hence lead to potential code size explosion. A code size explosion due to a particular implementation choice is undesirable, but a code size explosion imposed by the semantics is much more problematic. > I think it would be nice to have a lexical three-argument version of > pcase which specifies which variables are output values, treating the > remaining ones as input values, to make it easier to build > non-constant patterns. The design of `pcase` assumes you want to optimize away the tests that are common to the various patterns. That can't be done with dynamic patterns. > IOW, if you want to call a function with arguments determined by > pcase-like patterns, why not introduce pcase-call so something like > the following would work: > > (defun f (hello world) (cons hello world)) > > (let ((space " ") (hw "hello world")) > (pcase-call 'f ((concat hello space world) hw))) How do you intend to implement this? > As for duplicating the body, that is an implementation detail. You can > easily avoid it by producing > > (let ((name name)) > (cond ((symbolp V) X) > (progn (setq name V) X))) So it's more like my option of returning nil, except it would return the value of a surrounding `name` variable? That could be done, but I'm not convinced it'd be more often useful. > disallowing the modification of name in X. That's rather hard to do (and I don't see what would be the benefit here). >> The "intended" behavior instead would be to behave like >> >> (cond >> ((symbolp V) (let ((name nil)) (let ((foo 'bar)) name))) >> (t (let ((name V)) (let ((foo 'bar)) name)))) >> >> That's already the behavior you get if you switch the two: >> >> (macroexpand '(pcase V >> ((or (and (pred foo) name) (pred symbolp)) >> (let ((foo 'bar)) name)))) >> => >> (let* ((pcase-0 (lambda (name) (let ((foo 'bar)) name)))) >> (cond ((foo V) (funcall pcase-0 V)) >> ((symbolp V) (funcall pcase-0 nil)) >> (t nil))) > > I don't see where the nil comes from, or why it's a useful choice for > a default value. It comes from the absence of a binding for `name` and was chosen because nil is the standard default value in Elisp. It comes from this code in pcase.el: (let ((args (mapcar (lambda (pa) (let ((v (assq (car pa) vars))) (setq vars (delq v vars)) (cdr v))) prevvars))) ;; If some of `vars' were not found in `prevvars', that's ;; OK it just means those vars aren't present in all ;; branches, so they can be used within the pattern ;; (e.g. by a `guard/let/pred') but not in the branch. ;; FIXME: But if some of `prevvars' are not in `vars' we ;; should remove them from `prevvars'! `(funcall ,res ,@args))))))) The computation of `args` searches in `vars` for the bindings expected by the branch (stored in `prevvars` the first time we encountered that branch). The assq+cdr will return nil if a var from `prevvars` isn't found in `vars`. >> the fact that the behavior depends on the order of elements in `or` is >> an undesirable side effect of the implementation technique. > It also depends on the complexity of the branch. > It seems to me there are at least three consistent ways of behaving > (throw an error, bind name to nil, bind name to name), with an > inconsistent fourth way being what's currently implemented. The current implementation amounts to "we should signal an error but we don't bother doing so and just warn against it in the manual". Patch welcome ;-) >> I don't know of a simple implementation. > Here's my better-than-nothing attempt. I don't think that's complex; > if anything, it's too trivial. So you give it a search-based semantics. The problem with it for me is that if we turn `(,a ,@b) into (append `(,a) b) the pcase match will take a lot more time than the equivalent `(,a . ,b) Of course, you can try and handle these "easy" cases more efficiently, but then your ,@ will sometimes be very cheap and sometimes very expensive (depending on when an optimization can be applied), which I think is a misfeature (it's for this same reason that I dislike CL keyword arguments for functions). I think it's fine to have such a search-based `append` (because it's "reliably expensive") but I'd rather not automatically use it for ,@ [ BTW, you don't need (nor want) `eval` in your definition. ] Stefan