From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Chris K. Jester-Young" Newsgroups: gmane.lisp.guile.devel Subject: Re: SRFI 41 for Guile Date: Fri, 16 Dec 2011 01:45:49 -0500 Message-ID: <20111216064549.GA18181@lotus.destinee.acro.gen.nz> References: <20111206193728.GB14416@yarrow.destinee.acro.gen.nz> <877h1zns7c.fsf@gnu.org> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Trace: dough.gmane.org 1324017976 18001 80.91.229.12 (16 Dec 2011 06:46:16 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Fri, 16 Dec 2011 06:46:16 +0000 (UTC) To: guile-devel@gnu.org Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Fri Dec 16 07:46:12 2011 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1RbRYZ-0004Nk-RX for guile-devel@m.gmane.org; Fri, 16 Dec 2011 07:46:12 +0100 Original-Received: from localhost ([::1]:48538 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbRYZ-0002Qs-5z for guile-devel@m.gmane.org; Fri, 16 Dec 2011 01:46:11 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:38996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbRYV-0002QA-HX for guile-devel@gnu.org; Fri, 16 Dec 2011 01:46:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RbRYU-0007Kj-3l for guile-devel@gnu.org; Fri, 16 Dec 2011 01:46:07 -0500 Original-Received: from mail-qy0-f169.google.com ([209.85.216.169]:47290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RbRYT-0007KN-TD for guile-devel@gnu.org; Fri, 16 Dec 2011 01:46:06 -0500 Original-Received: by qcsd17 with SMTP id d17so1872275qcs.0 for ; Thu, 15 Dec 2011 22:46:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-type:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=57oK/Mm+Ll7xYJZdVHX2L7f8oqlupbjoqRehnij/pX8=; b=Ww38Ppe5V/UCl22qFpRmh1mTK3/wHbs4Smg2wZml8Em3yp9/+jIQuBO9devW9u3XUZ Es/PIOdouMt8/FFzrXyJlHQx5mDBlsxE4OWM/72HhCt9NMsxkfwJv0/WHOxJT4NEPgRS kdKaseOav6/hhLrkP1tosK9wnU2MSwyzcwGoc= Original-Received: by 10.229.137.21 with SMTP id u21mr941529qct.23.1324017964907; Thu, 15 Dec 2011 22:46:04 -0800 (PST) Original-Received: from lotus.destinee.acro.gen.nz (cpe-069-134-102-132.nc.res.rr.com. [69.134.102.132]) by mx.google.com with ESMTPS id r10sm17828806qaz.7.2011.12.15.22.46.03 (version=SSLv3 cipher=OTHER); Thu, 15 Dec 2011 22:46:03 -0800 (PST) Mail-Followup-To: guile-devel@gnu.org Content-Disposition: inline In-Reply-To: <877h1zns7c.fsf@gnu.org> User-Agent: Mutt/1.5.20 (2009-06-14) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 209.85.216.169 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:13125 Archived-At: On Wed, Dec 14, 2011 at 03:28:07PM +0100, Ludovic Courtès wrote: > Could you integrate it in the Guile tree in a branch, as you proposed on > IRC? It would make it easier to review it. Indeed. I might only get to do that in the weekend, and with Christmas and all that coming up, I can't even promise it'd get done this weekend. > What you be OK with assigning copyright for your changes to the FSF? Of course! Just for the record, I've already signed all the necessary paperwork, but you knew that already. The copyright notices will get updated when they go "in-tree". > Also, to what extent is the reference implementation modified? If the > modifications are well isolated, it would be nice to see if we could > import the upstream files unmodified, and then just “patch” them at > compile-time as is done for match.scm, sxml-match.scm, lalr, and a few > others. The changes are very extensive. Almost no function was left intact, except perhaps the stream-of macro. (It stuck out as one that I wasn't able to "do better" on.) I chose to do this because I don't expect the reference implementation to receive any maintenance at this stage. This is in contrast to the modules you mentioned above, where you do want to touch base with upstream from time to time, so leaving them unchanged would reduce the maintenance burden. In the case of SRFI 41, I would expect that we'd have an independent implementation that would become its own thing, and eventually diverge more and more from the reference implementation as we find things to improve. Again, this is because I feel the reference implementation is no longer maintained. The most important change is that any place where some function can use another module to do its work, I choose to use the external module to do so. A prime example is SRFIs. Whereas the reference implementation goes to extraordinary lengths to avoid depending on other SRFIs, I do the opposite, and use any and all SRFIs available in Guile that do the job. For example, I use Guile's SRFI 45 for lazy promises instead of replicating the whole thing as the reference implementation does. Other than that, for most functions, the changes followed a predictable pattern, which didn't change the logic at all, but rather just made the functions easier to read (for me, at least). In particular, the reference implementation uses this format: (define (stream-foo strm) (define (stream-foo strm) ...) (if (not (stream? strm) ERROR)) (stream-foo strm)) I use this format instead: (define (stream-foo strm) (must stream? strm) (stream-let recur ((strm strm)) ...)) The latter is better, in my view, because I don't have to remember that inside stream-foo, there's an internal binding for stream-foo that shadows the outer one. stream-foo means one thing only. stream-drop-while and stream-reverse are further refactored to use a new stream-do macro. All the eager-folding functions (stream->list, stream-drop, stream-fold, stream-length) are refactored to use a common stream-fold-aux function. This reduces code duplication quite a bit. For functions that take 1+ streams (stream-for-each, stream-map), I used case-lambda to both optimise for the one-stream case, and to block calling the function with no streams. The above covers changes that don't change the actual logic, they just reformulate it in a different way. There are also functions where the actual logic is changed, particularly stream-constant, stream-match, and stream-unfolds. Those do not bear any similarities to the reference implementation; they are indeed complete rewrites. You will want to review those in full. > Likewise, it may be possible to adapt the original test suite just by > defining macros that map the expected test suite API to Guile’s. This is hard, for several reasons. Guile's test framework has labels for each test, to easily identify which ones fail. The reference tests do not have such labels. Coming up with good labels is not something a macro can do, I believe. In particular, I don't think a stringification of the form under test is a good, human-readable label. Also, Guile's test framework has explicit functions for testing for expected exceptions. The reference tests do not. They simply catch exceptions, return the message string, and the tests simply expects the message string. It works in a pinch, but is nowhere as useful as Guile's pass-if-exception. There are a number of tests that test whether a given stream has the expected contents. The reference tests use stream->list to materialise the whole stream, then compare to the expected values. I don't do that. Instead, I wrote a stream-equal? function that stops as soon as the first mismatched element is found. > The idea is that it would make it easier to see how Guile’s > implementation differs from upstream, and to update Guile’s copy > eventually, if needed. As stated above, because my intention is for Guile's version to be independent of upstream, my stance is that if the tests do test everything in the reference test suite (and they do, and some more), and they all pass, then it's a valid implementation. When I first started the port, I wanted to be as faithful to the reference implementation as possible, just to reduce my work. However, when I realised that it basically reimplemented most everything already available in Guile modules, I decided that using the modules is more appropriate. Then updates to those modules would benefit SRFI 41 too. Case in point: (ice-9 match) still gets upstream updates. By using that module for matching, we get all its future benefits for free. > Also, what about moving all or most of the code in srfi-41.scm, instead > of having several helper modules? I don't oppose this per se, but I should state for the record what the purpose of the primitive/derived split is. (streams primitive), as it's called in SRFI 41, provide the basic operations needed to have streams. It's totally bloat-free, and if you want to create your own stream utility module, you would import just this module. (streams derived), as it's called in SRFI 41, provide utility functions and macros atop the (streams primitive) streams. Most end-users will want to import this module, but it's probably a bit bloaty for stream library authors to use. So, while I don't oppose moving all of the _implementation_ into the top-level (srfi srfi-41), it'd be good to retain the primitive/derived submodules that can be used to do selective imports of (srfi srfi-41). That way, people who want bloat-free can do so without having to rewrite their own import lists. (In so saying, (streams derived) isn't very useful without (streams primitive), so perhaps a (srfi srfi-41 derived) selective import module is probably not useful, and people should just import (srfi srfi-41) straight. But (srfi srfi-41 primitive) is still, IMHO, useful.) > (This has been partly discussed on IRC while I was writing this > message; I hope it’s still relevant. ;-)) Sure; if nothing else, this puts the whole conversation "on the record". :-) Cheers, Chris.