From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.bugs Subject: bug#29799: 24.5; cl-loop guard clause missing Date: Wed, 03 Jan 2018 19:34:51 +0900 Message-ID: <87lghfqlh0.fsf@gmail.com> References: <87d138beur.fsf@gmail.com> <874lo5tfpl.fsf@users.sourceforge.net> <871sj9tcdb.fsf@users.sourceforge.net> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1514975667 30284 195.159.176.226 (3 Jan 2018 10:34:27 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 3 Jan 2018 10:34:27 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) Cc: 29799@debbugs.gnu.org, monnier@iro.umontreal.ca To: Noam Postavsky Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Jan 03 11:34:23 2018 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eWgND-00071H-FQ for geb-bug-gnu-emacs@m.gmane.org; Wed, 03 Jan 2018 11:34:15 +0100 Original-Received: from localhost ([::1]:47944 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eWgPA-00088l-HI for geb-bug-gnu-emacs@m.gmane.org; Wed, 03 Jan 2018 05:36:16 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:34711) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eWgP2-00087D-8W for bug-gnu-emacs@gnu.org; Wed, 03 Jan 2018 05:36:09 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eWgOw-0005Jl-0g for bug-gnu-emacs@gnu.org; Wed, 03 Jan 2018 05:36:08 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:53380) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eWgOv-0005JW-T3 for bug-gnu-emacs@gnu.org; Wed, 03 Jan 2018 05:36:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eWgOv-0000ZT-Kh for bug-gnu-emacs@gnu.org; Wed, 03 Jan 2018 05:36:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Tino Calancha Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 03 Jan 2018 10:36:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 29799 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 29799-submit@debbugs.gnu.org id=B29799.15149757072128 (code B ref 29799); Wed, 03 Jan 2018 10:36:01 +0000 Original-Received: (at 29799) by debbugs.gnu.org; 3 Jan 2018 10:35:07 +0000 Original-Received: from localhost ([127.0.0.1]:33828 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eWgO3-0000YG-1L for submit@debbugs.gnu.org; Wed, 03 Jan 2018 05:35:07 -0500 Original-Received: from mail-wm0-f41.google.com ([74.125.82.41]:35362) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1eWgNz-0000Xh-V7 for 29799@debbugs.gnu.org; Wed, 03 Jan 2018 05:35:04 -0500 Original-Received: by mail-wm0-f41.google.com with SMTP id a79so1893071wma.0 for <29799@debbugs.gnu.org>; Wed, 03 Jan 2018 02:35:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version; bh=okLSaCW4LZT062qbP/if8OGaMofCyAMtzp4n6EIVGjA=; b=XxvMXxQ/nEFzbRs1k0jKHEfAOg1zNpdIIfg8Q46yWUIy+NQo7A9Tjzn70uU2vmSEZ2 poW3WnURCOFX3xiqWtjD7iom7Uxz1m/EjPocuZtsCYofx+E3tBAARiICChuT+JNtr8OZ pFE34k7HMiYj+1l1XdN+5hzZRJkrQtE2K7o6md5VNJ0wJyessfBZFgwx+UNemQBHH3Fi /CXoV3FwMZbKDwb+hMzuUtEsjgZpqj9Nly9XOCeI4VGloaaHZRWXuxIz1COVEF8RJxFL sWjyoqSK7BRhtZ2b3LcKmW+t13SkRyMW2BBN1GeeslQxSZw29cL5nvHdUa8ncx26b/L4 aEYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references :user-agent:date:message-id:mime-version; bh=okLSaCW4LZT062qbP/if8OGaMofCyAMtzp4n6EIVGjA=; b=DOnB9cJiZ6kgUZe+E+IEbmKf0lwTrs205l5SXgRffUnuZ9I8xruUaD0KHw8olYdAbN /BHR4rGDlqGf4tOytb5HqV/Dz4rqHm4iepyD5z32M8KOy5ZrzSeQ+yl6h4oU2tzGpd0E NlQp11xtpQ6el63d1G40rBhlAGjZwmbPJa0gBcPv1KtwbtVMSrbH+SC5k/uftSkYzHsx MKDdmt4h+8AxitQQDIcDCTWgtu+FKaHylPtL+K4HxAj+EaJB1YUJgn5ab6eMB5g43Qq1 mzdTlo14L6s43zsIGtcFhq2kX0B6V6SwpbZIi2/S6fPfsgljbL14UKwWg64/nEkuJdbl BMPw== X-Gm-Message-State: AKGB3mLRkHFo1bQK2YAruALqVu9KF4YQj5VIj1oUtQBPs4CTzllfNJZB 8XSBkWNIKEa/n8Wh6j5dxzU= X-Google-Smtp-Source: ACJfBotTNMord2P1kfl00K5rLOfA3XiR4V6jqd1t12LwF4Bvc+BJEVukeV6EiPr8ZMb/y68KFop2dA== X-Received: by 10.28.128.214 with SMTP id b205mr1094830wmd.82.1514975698130; Wed, 03 Jan 2018 02:34:58 -0800 (PST) Original-Received: from calancha-pc (228.red-83-40-68.dynamicip.rima-tde.net. [83.40.68.228]) by smtp.gmail.com with ESMTPSA id s187sm423478wmf.16.2018.01.03.02.34.55 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 03 Jan 2018 02:34:56 -0800 (PST) In-Reply-To: <871sj9tcdb.fsf@users.sourceforge.net> (Noam Postavsky's message of "Mon, 01 Jan 2018 17:58:40 -0500") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 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.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:141742 Archived-At: Noam Postavsky writes: > Noam Postavsky writes: > >> I don't understand why the "then" step is put at the of the loop. The >> following patch (commenting out the "ands" branch) avoids doing that, >> and fixes this bug. But presumably there is some reason for having this >> code in the first place? I guess some more complicated example would be >> needed to show why this naive fix won't work. > > Oh, 'make -C test cl-macs-tests' provides some. And I see we've in fact > been over this naive solution before: > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=6583#28 Bug#6583 requires more thinking; I couldn't find a satisfactory fix for it. For Bug#29799 I propose the patch below: * It adds a new variable `cl--loop-guard-cond' * In a for clause, rigth after update the loop var, check if the loop condition is still valid before update the remaining variables. AFAIS, this is similar to the CL expansions for these cases. --8<-----------------------------cut here---------------start------------->8--- commit 25fb3aad45ea3c545c6389c4f7bb6f1a76ebffe8 Author: Tino Calancha Date: Wed Jan 3 19:15:14 2018 +0900 Fix #Bug#29799 * lisp/emacs-lisp/cl-macs.el (cl--loop-guard-cond): New variable. (cl--parse-loop-clause): Set it non-nil if the loop contains a for/as clause. (cl-loop): After update the loop variable, update other variables only if cl--loop-guard-cond is non-nil. * test/lisp/emacs-lisp/cl-macs-tests.el (cl-macs-loop-for-as-equals-and): New test. diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el index f5311041cc..db1b811f38 100644 --- a/lisp/emacs-lisp/cl-macs.el +++ b/lisp/emacs-lisp/cl-macs.el @@ -892,7 +892,7 @@ cl--loop-initially (defvar cl--loop-name) (defvar cl--loop-result) (defvar cl--loop-result-explicit) (defvar cl--loop-result-var) (defvar cl--loop-steps) -(defvar cl--loop-symbol-macs) +(defvar cl--loop-symbol-macs) (defvar cl--loop-guard-cond) (defun cl--loop-set-iterator-function (kind iterator) (if cl--loop-iterator-function @@ -961,7 +961,7 @@ cl-loop (cl--loop-accum-var nil) (cl--loop-accum-vars nil) (cl--loop-initially nil) (cl--loop-finally nil) (cl--loop-iterator-function nil) (cl--loop-first-flag nil) - (cl--loop-symbol-macs nil)) + (cl--loop-symbol-macs nil) (cl--loop-guard-cond nil)) ;; Here is more or less how those dynbind vars are used after looping ;; over cl--parse-loop-clause: ;; @@ -996,7 +996,22 @@ cl-loop (list (or cl--loop-result-explicit cl--loop-result)))) (ands (cl--loop-build-ands (nreverse cl--loop-body))) - (while-body (nconc (cadr ands) (nreverse cl--loop-steps))) + (while-body + (nconc + (cadr ands) + (if (or (not cl--loop-guard-cond) (not cl--loop-first-flag)) + (nreverse cl--loop-steps) + ;; Right after update the loop variable ensure that the loop condition, + ;; i.e. (car ands), is still satisfied; otherwise do not + ;; update other variables (#Bug#29799). + ;; (last cl--loop-steps) updates the loop var + ;; (car (butlast cl--loop-steps)) sets `cl--loop-first-flag' nil + ;; (nreverse (cdr (butlast cl--loop-steps))) sets the + ;; remaining variables. + (append (last cl--loop-steps) + `((and ,(car ands) + ,@(nreverse (cdr (butlast cl--loop-steps))))) + `(,(car (butlast cl--loop-steps))))))) (body (append (nreverse cl--loop-initially) (list (if cl--loop-iterator-function @@ -1500,10 +1515,11 @@ cl--parse-loop-clause ,(cl--loop-let (nreverse loop-for-sets) 'setq ands) t) cl--loop-body)) - (if loop-for-steps - (push (cons (if ands 'cl-psetq 'setq) - (apply 'append (nreverse loop-for-steps))) - cl--loop-steps)))) + (when loop-for-steps + (setq cl--loop-guard-cond t) + (push (cons (if ands 'cl-psetq 'setq) + (apply 'append (nreverse loop-for-steps))) + cl--loop-steps)))) ((eq word 'repeat) (let ((temp (make-symbol "--cl-var--"))) diff --git a/test/lisp/emacs-lisp/cl-macs-tests.el b/test/lisp/emacs-lisp/cl-macs-tests.el index 575f170af6..2aab002964 100644 --- a/test/lisp/emacs-lisp/cl-macs-tests.el +++ b/test/lisp/emacs-lisp/cl-macs-tests.el @@ -497,4 +497,12 @@ vconcat (vector (1+ x))) [2 3 4 5 6]))) + +(ert-deftest cl-macs-loop-for-as-equals-and () + "Test for https://debbugs.gnu.org/29799 ." + (let ((arr (make-vector 3 0))) + (should (equal '((0 0) (1 1) (2 2)) + (cl-loop for k below 3 for x = k and z = (elt arr k) + collect (list k x)))))) + ;;; cl-macs-tests.el ends here --8<-----------------------------cut here---------------end--------------->8---