From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Daniel Dinnyes Newsgroups: gmane.lisp.guile.devel Subject: Re: Hygienic rewrite of (ice-9 expect) Date: Tue, 23 May 2023 11:48:55 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="00000000000078e89505fc5a2433" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="35493"; mail-complaints-to="usenet@ciao.gmane.io" To: guile-devel@gnu.org Original-X-From: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Tue May 23 12:49:55 2023 Return-path: Envelope-to: guile-devel@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 1q1PaV-0008yU-Hi for guile-devel@m.gmane-mx.org; Tue, 23 May 2023 12:49:55 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1q1PaI-00014f-ND; Tue, 23 May 2023 06:49:43 -0400 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 1q1PaH-00014N-4U for guile-devel@gnu.org; Tue, 23 May 2023 06:49:41 -0400 Original-Received: from mail-wm1-x32b.google.com ([2a00:1450:4864:20::32b]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1q1PaC-0004bT-CE for guile-devel@gnu.org; Tue, 23 May 2023 06:49:40 -0400 Original-Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-3f60dfc5f93so2327685e9.2 for ; Tue, 23 May 2023 03:49:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684838973; x=1687430973; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=/vzcUDEAy2lGICVIkAG2mDBzkv3+/apBLvFORT5PGAw=; b=mI3bkM/eRXi11pGmZPe/IxdtMwDpHZCVDeuEGNcBwBz9iAtnJU0NMnunNlTAwAK8BP d+u8bhcfUZW3izl7WBJILCuJFmfF6qcRRs7NROx7zD348j9AjXeOl2UsoGMZBlVYfOni 4XbZP4gkMoRJjc6nmcJqH0qkYZLIts/bEGB427Tj/774nTwvPwW5mjd1T2UFKiFoX9E5 nzhEN6Yt/LPfpfWHLjcoTryLDn+H8g80TcyfqN2AHu7yP4RHPtE5af7q0uL/nKsGoz49 Sl8SNnwOBQmJK5XKcp8YLzRApCHTnVq3dXHJ9B5sUepT3wkCjmMJ9NTS+LBR7uie40ai laUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684838973; x=1687430973; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=/vzcUDEAy2lGICVIkAG2mDBzkv3+/apBLvFORT5PGAw=; b=UTgUQec0YBGu6qgx2JVhj5/RXr6K/ZvrhG77eQMEeJ4gDvd1t7hDKkHj6d6xmDiZiz V1iz/IKpb0pOji/xlNdM9XdKDiF6j9uc/v6oA+l3vXNEBJKtnIRxIsTG25SUJe62EaJR viX9R6eN4gk5RjWuzw82dwc/rET4/IUmEg+HLXKWvDswbjFWVb8Sl3+fPfwExVPTLU9w EdyOXApl6yZUnCfEmxv/ENF5uH9rRKLayV59Mt2I+XT6k2FwO3wyZYldIfdvbIZoL4qC v53WCQeunvjQYtEleLRjMwNtyvkeGxBfGJDqkdvIDCVEjmWwqmgoGWXvwh/jDv3O0Pva iVJQ== X-Gm-Message-State: AC+VfDwCkb+xPo7PKoxPheDDAWKDVMq2vh3hkDzFY8lFRMuM8+66uzCY HDOZwyoEu4mWe9PFm0qmlprl0hzZI+F8TxCQaAWDAkxBTes= X-Google-Smtp-Source: ACHHUZ6cJx5iApFI83vktWvQEgT2DLntKukMIDgGIhqPLkAFT1HnlHeklLW5JZd8ehFNc/jlxbRrAXBX/yeer9ie9Gk= X-Received: by 2002:a7b:cbc8:0:b0:3f4:28fd:83e0 with SMTP id n8-20020a7bcbc8000000b003f428fd83e0mr9660916wmi.31.1684838972709; Tue, 23 May 2023 03:49:32 -0700 (PDT) In-Reply-To: Received-SPF: pass client-ip=2a00:1450:4864:20::32b; envelope-from=dinnyesd@gmail.com; helo=mail-wm1-x32b.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.29 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-mx.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.lisp.guile.devel:21835 Archived-At: --00000000000078e89505fc5a2433 Content-Type: text/plain; charset="UTF-8" Need to add a correction: The new interact macro captures the expect-port for input if available, and uses it as the interact/output port too. This is unless an explicit interact/output port is passed as an optional argument, in which case the lines read from default current-input-port (using ice-9 readline) are then sent to this port instead. Cheers, Daniel On Sun, 21 May 2023 at 23:26, Daniel Dinnyes wrote: > Hi everyone, > > It seems this hasn't garnered much attention. > > Nevertheless, meanwhile I've realised the code had a few bugs, in > particular around the backwards-compatibility flag. > > I've done a major rewrite: > > - fixed couple of bugs I found in the original implementation > - made implementation more robust, simpler, and backwards-compatible: > - added new expect-chars macro to support the new > non-backwards-compatible matcher procedure signature. > - removed need for expect-pass-char? backwards-compatibility > feature flag. expect and expect-strings macros work as in ice-9. > - improved interact macro, to allow specifying separate > expect/input and interact/output ports. > > The code can be found here > . > > Thanks, > Daniel > > > On Sat, 15 Apr 2023 at 16:10, Daniel Dinnyes wrote: > >> Hi everyone, >> >> I am new to this mailing list, and writing this because I have >> encountered some issues with ice-9 expect >> >> : >> >> The procedural expect macro has a serious problem: >> >> This syntax expects a procedure as the test expression, with 2 arguments >> (actual content read, and boolean EOF flag). The primary problem is that >> when the test expression is not a identifier bound to such a procedure, but >> instead an expression which creates such a procedure (aka "factory"), then >> the expression will be evaluated anew for each character read from >> expect-port. This is a serious problem: when constructing that procedure >> has significant computational costs, or side effects (eg. opening a port to >> a new log file), as it can be seen in my code here >> . >> (This is a branch of some of my KVM-based E2E testing code, which uses the >> original expect macro implementation, just to demonstrate the problem). >> >> I had written a workaround for this problem, by binding the evaluation of >> the test expression outside the expansion of the expect macro (as it can be >> seen here >> ). >> The problem with this was, that it doesn't support the full capabilities of >> the original expect macro, and debilitates it by only allowing for a single >> conditional branch. Also, I haven't verified, but I suspicious it would >> possibly even break the behaviour for the => syntax of the expect-strings >> macro. >> >> To implement a more complete version of this (and also as an exercise to >> understand hygienic macros), I've attempted to rewrite my workaround using >> syntax-case. I've ran into some issues, and my implementation didn't >> expand as I expected. After some discussion on IRC, we've concluded >> that this was >> because the current (ice-9 expect) implementation is written using >> unhygienic defmacro syntax, which doesn't work well together with the >> hygienic syntax-case. It was suggested I should rather rewrite expect >> completely to be hygienic instead. >> >> I've eventually completed the rewrite using syntax-case here >> , and >> would be happy to contribute it back to the ice-9 module! >> I would point out the following about this implementation: >> >> 1. It works!!! I had some quite extensive E2E testing code written >> before here >> , >> which I think really stretches the possibilities with (ice-9 expect). >> Switching to the new implementation was simple, and works flawlessly! >> 2. It is backwards compatible! I've gone extra lengths to make sure >> the code would be a backwards compatible, a drop-in replacement. >> 3. Introduced new feature for the procedural expect macro, such that >> the second argument of the procedure gets passed the currently read char, >> or #f if it was an EOF character. There is multiple reasons this is >> superior to passing just a boolean eof? flag argument: >> 1. the same logic can be implemented as with the eof? boolean, just >> by negating the condition >> 2. passing the currently read char avoids having to do the >> inefficient operation to retrieve the currently read char by cutting off >> the last character from the end of the content. Currently it is necessary >> to do this, if the logic wants something additional to be done with the >> current char (eg. as it can be seen in my code here >> >> ) >> 3. one such case of additional use is making the >> expect-char-proc property obsolete, as the predicate procedure >> is now able to execute such logic too, besides other more advanced logging >> scenarios (like it can be seen in my code here >> : >> here besides logging everything to STDOUT, and also to a main log-file, I >> also log for each expect macro call the contents it tries to compare >> against into a separately named minor log-file (useful for debugging), and >> also to ensure that we actually do not write to STDOUT when using expect on >> multiple parallel processes) >> 4. To ensure that everything stays backwards compatible, this >> new feature is only enabled if the new expect-pass-char? >> binding is set to #t in the lexical context (like here >> ). >> Otherwise, by default this feature is disabled, and boolean eof? >> flag argument is passed just like before. >> 4. There is a better support for using default >> value/expressions for the various parameters the macros depend on in their >> lexical contexts. The old macro had to export default values defined in its >> module, so as to ensure the defaults are available. This either clutters >> the module's global context where (ice-9 expect) is used, or the >> defaults are not available when using something like ((ice-9 expect) >> #:select (expect expect-string)). The new defaults are calculated >> based on the lexical context where the macros are used, and expands into >> the correct default values when they are not defined. For example when >> expect-port is not defined in the lexical context, then the default value >> used is going to be the result of the (current-input-port) >> expression. I would like someone to review whether I've implemented this >> correctly, as it seems the current-input-port identifier default for >> expect-port gets captured in the expansion. Not sure why that happens. >> 5. The expect-strings macro supports the => syntax, which passes the >> output(s) of the test predicate to a procedure, instead of just evaluating >> a body. This is fully supported, but additionally, the => syntax now >> also works with the procedural expect macro too! I haven't verified if the >> original implementation did this too, but at least it wasn't described in >> the documentation! >> 6. As an added bonus, I have added a simplistic implementation for an >> interact macro. It uses threads to read from STDIO, and expects expect-port >> to be an input-output-port, so that it can output the lines read from STDIO >> into the same port. Not too advanced, as it doesn't support a full terminal >> interface, with autocomplete, etc... but works well enough for my use-case >> to interact with a KVM process via serial port. >> 7. mnieper said >> that I am not using syntax-case idiomatically, and my style is more in the >> form of a syntax-rules-based state-machine. >> I am quite happy with how it is currently, but would be open to be >> convinced if there is a superior way of expressing all this. >> 8. I currently have various tests sprinkled around my implementation, >> to manually demonstrate expansions, and executions. >> Also, I have added some automated unit tests for the bind-locally >> helper procedure, which I used for checking whether parameters are defined >> in the lexical scope. >> 9. I am not happy with how I am managing my project structure, and >> how my modules can reference each other (having to call add-to-load-path >> everywhere). >> Also, I think the way I am using srfi-64 tests is a bit bothersome, >> as they always get executed when my modules are loaded, and I have to >> globally disable logging to files. >> I would like some recommendations how to do this better! >> >> If the community likes this implementation, I would be happy to >> apply/rebase my changes onto the official repo, and then we can go from >> there! >> >> Best regards, >> Daniel Dinnyes >> >> >> --00000000000078e89505fc5a2433 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Need to add a correction:

The new interact macro captu= res the expect-port for input = if available, and uses it as the interact/output port too.
This i= s unless an explicit interact/output port is passed as an optional argument= , in which case the lines read from default current-input-port (using ice-9= readline) are then sent to this port instead.

Cheers,
Daniel

On Sun, 21 May 2023 at 23:26, Daniel Din= nyes <dinnyesd@gmail.com> w= rote:
Hi everyone,

It seems this hasn'= t garnered much attention.

Nevertheless, meanw= hile I've realised the code had a few bugs, in particular around the ba= ckwards-compatibility flag.

I've done a major = rewrite:
  • fixed couple of bugs I found in the original imp= lementation
  • made implementation more robust, simpler, and backwards= -compatible:
    • added new expect-chars macro to support the new non-backwards-compatible mat= cher procedure signature.
    • removed need for expect-pass-char? backwards-compatibility feature flag= . expect and expect-strings macros work as in ice-9.
  • improved interact ma= cro, to allow specifying separate expect/input and interact/output ports.
The code can be found here.
=
Thanks,
Daniel


On Sat, 15 Apr 2= 023 at 16:10, Daniel Dinnyes <dinnyesd@gmail.com> wrote:
Hi everyone,

I am new to this mailing list, and writing this because I= have encountered some issues with ice-9 expect= :

The procedural expect macro has a serious pr= oblem:

This syntax expects a procedure as the= test expression, with 2 arguments (actual content read, and boolean EOF fl= ag). The primary problem is that when the test expression is not a identifi= er bound to such a procedure, but instead an expression which creates such = a procedure (aka "factory"), then the expression will be evaluate= d anew for each character read from expect-port. This is a serious problem:= when constructing that procedure has significant computational costs, or s= ide effects (eg. opening a port to a new log file), as it can be seen in my= code here. (This is a branch of some of m= y KVM-based E2E testing code, which uses the original expect macro implemen= tation, just to demonstrate the problem).

I had wr= itten a workaround for this problem, by binding the evaluation of the test = expression outside the expansion of the expect macro (as it can be seen here). The problem with this was, that it doesn= 9;t support the full capabilities of the original expect macro, and debilit= ates it by only allowing for a single conditional branch. Also, I haven'= ;t verified, but I suspicious it would possibly even break the behaviour fo= r the =3D> syntax of the expect-strings macro.
<= div>
To implement a more complete version of this (and also a= s an exercise to understand hygienic macros), I've attempted to rewrite= my workaround using syntax-case. I've ran into some issues, and my implementation didn't expand = as I expected. After some discussion on IRC, we've concluded= that this was because the current (i= ce-9 expect) implementation is written using unhygienic defmacro syntax, which doesn't work w= ell together with the hygienic syntax= -case. It was suggested I should rather rewrite expect completely to= be hygienic instead.

I've eventually complete= d the rewrite using syntax-case here, and would be h= appy to contribute it back to the ice-9 module!
I would point out= the following about this implementation:
  1. It works!!! I h= ad some quite extensive E2E testing code written before here, which I think really stretches the possibilities with (ice-9 ex= pect). Switching to the new implementation was simple, and works flawlessly= !
  2. It is backwards compatible! I've gone extra lengths to ma= ke sure the code would be a backwards compatible, a drop-in replacement.
  3. Introduced new feature for the procedural expect macro, such that the second argument of the pr= ocedure gets passed the currently read char, or #f if it was an EOF charact= er. There is multiple reasons this is superior to passing just a boolean eo= f? flag argument:
      1. the same logic can be implemented as = with the eof? boolean, just by negating the condition
      2. passing the currently read char avoids having to do the inefficient operation to r= etrieve the currently read char by cutting off the last character from the = end of the content. Currently it is necessary to do this, if the logic want= s something additional to be done with the current char (eg. as it=20 can be seen in my code here)
      3. one such case of additional use is making the expect-char-proc property obsolete, as the predicate procedure= is now able to execute such logic too, besides other more advanced logging scenarios (like it can be seen in my code here: here besides logging everything to STDOUT, and also to a main log-file, I also log for each expect macro call the contents it tries to compare=20 against into a separately named minor log-file (useful for debugging), and = also to ensure that=20 we actually do not write to STDOUT when using expect on multiple parallel processes)
      4. To ensure that everything stays backwards compatible, t= his new feature is only enabled if the new expect-pass-char? binding is set to #t in the lexical context = (like here). Otherwise,= by default this feature is disabled, and boolean eof? flag argument is passed just like before.
        5. =
  4. There is a better support for using default value/= expressions for the various parameters the macros depend on in their lexica= l contexts. The old macro had to export default values defined in its modul= e, so as to ensure the defaults are available. This either clutters the mod= ule's global context where (ice-9= expect) is used, or the defaults are not available when using somet= hing like ((ice-9 expect) #:select (e= xpect expect-string)).=C2=A0 The new defaults are calculated based o= n the lexical context where the macros are used, and expands into the corre= ct default values when they are not defined. For example when expect-port i= s not defined in the lexical context, then the default value used is going = to be the result of the (current-inpu= t-port) expression. I would like someone to review whether I've = implemented this correctly, as it seems the current-input-port identifier default for expect-port gets captured in the expansion. Not s= ure why that happens.
  5. The expect-strings macro supports the =3D> syntax, which passes the = output(s) of the test predicate to a procedure, instead of just evaluating = a body. This is fully supported, but additionally, the =3D> syntax now also works with the procedural = expect macro too! I haven't verified if the original implementation did= this too, but at least it wasn't described in the documentation!
  6. As an added bonus, I have added a simplistic implementation for an i= nteract macro. It uses threads to read from STDIO, and expects expect-port = to be an input-output-port, so that it can output the lines read from STDIO= into the same port. Not too advanced, as it doesn't support a full ter= minal interface, with autocomplete, etc... but works well enough for my use= -case to interact with a KVM process via serial port.
  7. mnieper <= a href=3D"http://snailgeist.com/irc-logs/?message_id=3D136211" target=3D"_b= lank">said that I am not using syntax-case idiomatically, and my style = is more in the form of a syntax-rules= -based state-machine.
    I am quite happy with how it is currently, = but would be open to be convinced if there is a superior way of expressing = all this.
  8. I currently have various tests sprinkled around my implem= entation, to manually demonstrate expansions, and executions.
    Also, I ha= ve added some automated unit tests for the bind-locally helper procedure, which I used for checking wheth= er parameters are defined in the lexical scope.
  9. I am not happy with= how I am managing my project structure, and how my modules can reference e= ach other (having to call add-to-load-path everywhere).
    Also, I think th= e way I am using srfi-64 tests is a bit bothersome, as they always get exec= uted when my modules are loaded, and I have to globally disable logging to = files.
    I would like some recommendations how to do this better!
    10. If the community likes this implementation, I would be happy to apply= /rebase my changes onto the official repo, and then we can go from there!

    Best regards,
    Daniel Dinnyes


--00000000000078e89505fc5a2433--