From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Noam Postavsky Newsgroups: gmane.emacs.bugs Subject: bug#30626: 26.0.91; Crash when traversing a `stream-of-directory-files' Date: Sat, 25 May 2019 16:29:38 -0400 Message-ID: <87muja8eh9.fsf@gmail.com> References: <87inaiss6l.fsf@web.de> <6FCF6ACA-4F29-4B6B-BE9D-D7130C6E9495@gnu.org> <87fu5moe4c.fsf@web.de> <877eqyocro.fsf@web.de> <83zi3uz4nb.fsf@gnu.org> <87lgfd52by.fsf@gmail.com> <87bmg91ity.fsf@web.de> <87imv2rcs3.fsf@gmail.com> <87y33ybr1a.fsf@web.de> <877eay30qf.fsf@web.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="241589"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) Cc: Nicolas Petton , 30626@debbugs.gnu.org To: Michael Heerdegen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sat May 25 22:40:10 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hUdSc-0010ip-3z for geb-bug-gnu-emacs@m.gmane.org; Sat, 25 May 2019 22:40:10 +0200 Original-Received: from localhost ([127.0.0.1]:46116 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUdSb-0002mU-6v for geb-bug-gnu-emacs@m.gmane.org; Sat, 25 May 2019 16:40:09 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:46761) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUdS8-0002LN-Je for bug-gnu-emacs@gnu.org; Sat, 25 May 2019 16:39:42 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hUdIp-0004BM-Ek for bug-gnu-emacs@gnu.org; Sat, 25 May 2019 16:30:04 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:36195) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hUdIp-0004BD-9X for bug-gnu-emacs@gnu.org; Sat, 25 May 2019 16:30:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hUdIp-0001pv-2g for bug-gnu-emacs@gnu.org; Sat, 25 May 2019 16:30:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Noam Postavsky Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 25 May 2019 20:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 30626 X-GNU-PR-Package: emacs Original-Received: via spool by 30626-submit@debbugs.gnu.org id=B30626.15588161927014 (code B ref 30626); Sat, 25 May 2019 20:30:02 +0000 Original-Received: (at 30626) by debbugs.gnu.org; 25 May 2019 20:29:52 +0000 Original-Received: from localhost ([127.0.0.1]:49739 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hUdIb-0001ov-4C for submit@debbugs.gnu.org; Sat, 25 May 2019 16:29:49 -0400 Original-Received: from mail-it1-f173.google.com ([209.85.166.173]:34065) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hUdIY-0001oT-0j; Sat, 25 May 2019 16:29:47 -0400 Original-Received: by mail-it1-f173.google.com with SMTP id g23so13858614iti.1; Sat, 25 May 2019 13:29:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=kIM6Fa9//Vdn1wdYuCP6FshnC1yhszyFFhcTByrSYvI=; b=VZS5qZeCe94dd97f6rHUFoB+5kNy8/OmHW30bVaah3hX7MdEgjYTdYiD3ueRfAzFQg /VImI5aPExMeNUVLaAP+iaRcTA9/EF1ovLjEHMznWJK1CniV+zsH5B9AP6MwManDsAg3 Dx63m0d/icHxaIjQUmTzluDziE+E9SoimNUUlQNeQxnDe4qA8q22lyXqPU0B8iB3VIqp RQrp3dHqfv7OmDBXrZbEizj2dhk6WsZuvqM3ncS4hDAGeEZy23XXnuBpbo0nqLVBWTv6 YilVL4kWRZlZSdIobvO7kEcKXIiX3iWxuL5TKXCKvW+9DY3AntNN/d/uB68H1FW+C8U6 /1PA== 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:references:date:in-reply-to :message-id:user-agent:mime-version; bh=kIM6Fa9//Vdn1wdYuCP6FshnC1yhszyFFhcTByrSYvI=; b=Ujp2UTeGvgQCpXkrY8ovDu+Fqa9ykrfuY0n7j8iHWoP612JMboOCfCz8OXAjyFK8Pr sO3JMN3jXW2pKpt0xYlvT5NHl0C2W+ardBGeFsZlx8QWw3Fxe7YKHrsFiUyS4we+KBlQ C3Vn1Oh2Es9kGGoFXaZSOHDHp1nWT8OdmP3jTPGC99T9GTIZoz5ZnHUOz/Xs0fATqXd3 ueLi5oQBr6IPBRCAIoOBPLX1rSV7VNpv/J9i8FxBPMQFI02PgXNnb9CQnB5lqN7gvjXG LzstbaZzB81nnoFFBBZsgCMby5mKAwproa7ndPDi+fXfotriCYSByLRfWu/bH1wxDPJ3 86Gw== X-Gm-Message-State: APjAAAVhTSKIZePqUrZ/UXBOO2psYjQda98Cu3hGYhxtKVApv75ufpi5 zpKDrxRfT02oum8xjqqQ2jl6JFoQ X-Google-Smtp-Source: APXvYqxjFf/zCTsd/RLvppzB/C9rcf+KS2lYmbBcl9bjLkCFFZlXJvvgdw/2g61NFJ/JYTTdREJTNw== X-Received: by 2002:a05:660c:917:: with SMTP id s23mr22663146itj.166.1558816180112; Sat, 25 May 2019 13:29:40 -0700 (PDT) Original-Received: from minid (cbl-45-2-119-34.yyz.frontiernetworks.ca. [45.2.119.34]) by smtp.gmail.com with ESMTPSA id b6sm712411iok.71.2019.05.25.13.29.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 25 May 2019 13:29:39 -0700 (PDT) In-Reply-To: <877eay30qf.fsf@web.de> (Michael Heerdegen's message of "Fri, 10 May 2019 15:20:08 +0200") 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: 209.51.188.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:159771 Archived-At: --=-=-= Content-Type: text/plain tags 30626 + patch quit Michael Heerdegen writes: >> > - stream-make should use cons instead of list (or maybe a struct?). >> >> I think cons would be ok. Would a struct make things slower? A struct might be slower, and cons has the advantage that the print output is more readable for humans too. E.g., with this code: (let ((s (stream-range 1 5))) (stream-flush s) s) ;; Using cons (patch in this message): (--stream-evald-- 1 --stream-evald-- 2 --stream-evald-- 3 --stream-evald-- 4 --stream-evald--) ;; Using list (previous patch): (--stream-evald-- (1 --stream-evald-- (2 --stream-evald-- (3 --stream-evald-- (4 --stream-evald-- nil))))) ;; I guess using a struct would look something like this: #(--stream-evald-- (1 . #(--stream-evald-- (2 . #(--stream-evald-- (3 . #(--stream-evald-- (4 . #(--stream-evald-- nil))))))))) ;; Using list with thunk (current, v2.2.4) (--stream-- #[256 "\211\203\007\0\303\242\207\303\242\204 \0\304\300\242\305\300\242\302\242\\\301\302\242#B\240\210\303\306\240\210\304\242\207" [(1) 5 (1) (t) ((1 --stream-- #[256 "\211\203\007\0\303\242\207\303\242\204 \0\304\300\242\305\300\242\302\242\\\301\302\242#B\240\210\303\306\240\210\304\242\207" [(2) 5 (1) (t) ((2 --stream-- #[256 "\211\203\007\0\303\242\207\303\242\204 \0\304\300\242\305\300\242\302\242\\\301\302\242#B\240\210\303\306\240\210\304\242\207" [(3) 5 (1) (t) ((3 --stream-- #[256 "\211\203\007\0\303\242\207\303\242\204 \0\304\300\242\305\300\242\302\242\\\301\302\242#B\240\210\303\306\240\210\304\242\207" [(4) 5 (1) (t) ((4 --stream-- #[256 "\211\203\007\0\300\242\207\300\242\204\024\0\301\302\240\210\300\303\240\210\301\242\207" [(t) (nil) nil t] 3 " (fn &optional CHECK)"])) stream-range t] 7 " (fn &optional CHECK)"])) stream-range t] 7 " (fn &optional CHECK)"])) stream-range t] 7 " (fn &optional CHECK)"])) stream-range t] 7 " (fn &optional CHECK)"]) >> > - stream-empty should just be a constant. >> >> Dunno if there are cases where this would be problematic, but I guess we >> could do this as well. I've done this in the patch below. Passes all the tests, and I can't see why it would be problematic. > @Nicolas: Do you want us to care about this or do you want to have a > look yourself? I don't want to hurry, I just don't want this to be > forgotten. If you say you have time in four months, it's still ok. Not getting any response; I'll wait another week for comments and then push. --=-=-= Content-Type: text/plain Content-Disposition: attachment; filename=0001-Drop-forced-lambda-s-from-stream-Bug-30626.patch Content-Description: patch >From 7b126616c87bf034c933de711befcd80a7ada3bb Mon Sep 17 00:00:00 2001 From: Noam Postavsky Date: Wed, 24 Apr 2019 22:51:19 -0400 Subject: [PATCH] Drop forced lambda's from stream (Bug#30626) Let the stream id distinguish between forced and unforced stream values. When the value is forced, replace the lambda with its result. This lets the lambda and anything it references be garbage collected. Change the representation of a stream from (--stream-- THUNK) to (--stream-fresh-- . (lambda () VALUE)) or (--stream-evald . VALUE). * packages/stream/stream.el (stream--identifier): Remove. (stream--fresh-identifier, stream--evald-identifier): New constants to replace it. (streamp): Check for new constants. (stream-make): Use cons and lambda instead of list and thunk-delay. (stream--force): New function. (stream-empty-p, stream-first, stream-rest): Use it. (stream-empty): New constant, return it from the function instead of creating a new one each time. * packages/stream/tests/stream-tests.el (stream-to-list): Remove. (stream-list-test): Use seq-into instead. --- packages/stream/stream.el | 41 +++++++++++++++++++++++++---------- packages/stream/tests/stream-tests.el | 12 ++-------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/packages/stream/stream.el b/packages/stream/stream.el index 3f6bc4b5b..9f73e8b86 100644 --- a/packages/stream/stream.el +++ b/packages/stream/stream.el @@ -1,6 +1,6 @@ ;;; stream.el --- Implementation of streams -*- lexical-binding: t -*- -;; Copyright (C) 2016 Free Software Foundation, Inc. +;; Copyright (C) 2016-2019 Free Software Foundation, Inc. ;; Author: Nicolas Petton ;; Keywords: stream, laziness, sequences @@ -41,7 +41,7 @@ ;; - ... ;; ;; All functions are prefixed with "stream-". -;; All functions are tested in test/automated/stream-tests.el +;; All functions are tested in tests/stream-tests.el ;; ;; Here is an example implementation of the Fibonacci numbers ;; implemented as in infinite stream: @@ -65,18 +65,30 @@ (eval-when-compile (require 'cl-lib)) (require 'seq) -(require 'thunk) (eval-and-compile - (defconst stream--identifier '--stream-- - "Symbol internally used to identify streams.")) + (defconst stream--fresh-identifier '--stream-fresh-- + "Symbol internally used to streams whose head was not evaluated.") + (defconst stream--evald-identifier '--stream-evald-- + "Symbol internally used to streams whose head was evaluated.")) (defmacro stream-make (&rest body) "Return a stream built from BODY. BODY must return nil or a cons cell whose cdr is itself a stream." (declare (debug t)) - `(list ',stream--identifier (thunk-delay ,@body))) + `(cons ',stream--fresh-identifier (lambda () ,@body))) + +(defun stream--force (stream) + "Evaluate and return the first cons cell of STREAM. +That value is the one passed to `stream-make'." + (cond + ((eq (car-safe stream) stream--evald-identifier) + (cdr stream)) + ((eq (car-safe stream) stream--fresh-identifier) + (setf (car stream) stream--evald-identifier) + (setf (cdr stream) (funcall (cdr stream)))) + (t (signal 'wrong-type-argument (list 'streamp stream))))) (defmacro stream-cons (first rest) "Return a stream built from the cons of FIRST and REST. @@ -159,24 +171,29 @@ (defun stream-range (&optional start end step) (defun streamp (stream) "Return non-nil if STREAM is a stream, nil otherwise." - (eq (car-safe stream) stream--identifier)) + (let ((car (car-safe stream))) + (or (eq car stream--fresh-identifier) + (eq car stream--evald-identifier)))) + +(defconst stream-empty (cons stream--evald-identifier nil) + "The empty stream.") (defun stream-empty () - "Return a new empty stream." - (list stream--identifier (thunk-delay nil))) + "Return the empty stream." + stream-empty) (defun stream-empty-p (stream) "Return non-nil if STREAM is empty, nil otherwise." - (null (thunk-force (cadr stream)))) + (null (cdr (stream--force stream)))) (defun stream-first (stream) "Return the first element of STREAM. Return nil if STREAM is empty." - (car (thunk-force (cadr stream)))) + (car (stream--force stream))) (defun stream-rest (stream) "Return a stream of all but the first element of STREAM." - (or (cdr (thunk-force (cadr stream))) + (or (cdr (stream--force stream)) (stream-empty))) (defun stream-append (&rest streams) diff --git a/packages/stream/tests/stream-tests.el b/packages/stream/tests/stream-tests.el index 021ed65cf..7487ef69b 100644 --- a/packages/stream/tests/stream-tests.el +++ b/packages/stream/tests/stream-tests.el @@ -1,6 +1,6 @@ ;;; stream-tests.el --- Unit tests for stream.el -*- lexical-binding: t -*- -;; Copyright (C) 2015, 2017 Free Software Foundation, Inc. +;; Copyright (C) 2015, 2017-2019 Free Software Foundation, Inc. ;; Author: Nicolas Petton @@ -29,14 +29,6 @@ (require 'stream) (require 'generator) (require 'cl-lib) -(defun stream-to-list (stream) - "Eagerly traverse STREAM and return a list of its elements." - (let (result) - (seq-do (lambda (elt) - (push elt result)) - stream) - (reverse result))) - (ert-deftest stream-empty-test () (should (streamp (stream-empty))) (should (stream-empty-p (stream-empty)))) @@ -240,7 +232,7 @@ (ert-deftest stream-range-test () (ert-deftest stream-list-test () (dolist (list '(nil '(1 2 3) '(a . b))) - (should (equal list (stream-to-list (stream list)))))) + (should (equal list (seq-into (stream list) 'list))))) (ert-deftest stream-seq-subseq-test () (should (stream-empty-p (seq-subseq (stream-range 2 10) 0 0))) -- 2.11.0 --=-=-=--