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: Sun, 21 May 2023 23:26:57 +0100 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="000000000000175be605fc3ba914" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="4179"; 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 Mon May 22 00:28:07 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 1q0rX4-0000rK-JO for guile-devel@m.gmane-mx.org; Mon, 22 May 2023 00:28:06 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1q0rWe-0006uW-TA; Sun, 21 May 2023 18:27:40 -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 1q0rWd-0006uE-Ti for guile-devel@gnu.org; Sun, 21 May 2023 18:27:39 -0400 Original-Received: from mail-wm1-x334.google.com ([2a00:1450:4864:20::334]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1q0rWb-00051f-1u for guile-devel@gnu.org; Sun, 21 May 2023 18:27:39 -0400 Original-Received: by mail-wm1-x334.google.com with SMTP id 5b1f17b1804b1-3f509ec3196so17838465e9.1 for ; Sun, 21 May 2023 15:27:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1684708054; x=1687300054; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date:message-id:reply-to; bh=EPlMO85TSz7OrTanf9q/0SC+Av/G0o5g+u6wYUcFv9o=; b=jYCP0YKGpTSwwvoU2++AP25Pr2VVnY7pHx/RmfFBtfkR5iK2l2nVl0AwAiG78AHE/V GRIdizVbdyGvlanlEtu9RYxixz3KAEDkyOBxEYqnKAcwWvqtu8lW21YFx7hUL5vGj4cz K0ysvQTLxD+HFjIuQzP0SeUimsXCSBjerjJ0hvLm/Yh1bjcHjSiSYX3VO9Ck/gFSXvs3 EJW9tUOVIjBm+9XAsjrIyPmgEZoC3r1YoCiymVuKEieEK7HoYDylP2ZBKmVrGVTnqsvb vH5idhWz0HLVn06nEHtNqxZRxunWbCpdpbd1djIGOArOkh2ZoCPK0V8WIDOKUe6L7POC JZFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684708054; x=1687300054; 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=EPlMO85TSz7OrTanf9q/0SC+Av/G0o5g+u6wYUcFv9o=; b=YAZivQpYXsAWs6zxp/l3fpmC2XGFUBvMCBpt443GazdwtsTJ6Hor5dF5IfI84pDZXj 6BGH+wZeng+PcPVxrVKqRk5NvKWxqYglQjINIg1uWJ0uvqdCEuVOcl1w06PZ93q3g4om zXQLjT2lwNDCqbK7rLjCBC2icR9bRVsoVz4o6/kR8s6ctE0Y80IEjbmnVYH2FeqbjFDh GtD7K0aeiwyW6jYeoxSLVidbIWDL+I9D8trxgDkHMDsE+GUw7frozMjx+R+/srdHodj8 WmGDOPvhJBHniBLSO/QotRRh4xNG/AN7ZKwYrPvZR37lYDnfNTqsnPd04hkIBm4tIScX gc5A== X-Gm-Message-State: AC+VfDx8bNnf2Yn4gTLlQ5+pPzs8Jr4QH92jCno0xlewngn3j85iKUgU 39cBZne1fj0wSUgdypdBNlDpHnIBg8+EEwwv6sYDtwGvNHE= X-Google-Smtp-Source: ACHHUZ4XTHMd+LxGPfI8Q+UTZoy5P+Q7A6fOQ/jRDoO8EIMx/JcRHrXeJu7xabEx3GSnQ7DY5fiOT7awq69JBIBFV2g= X-Received: by 2002:a05:600c:da:b0:3f4:f113:d6c8 with SMTP id u26-20020a05600c00da00b003f4f113d6c8mr5205259wmm.20.1684708053702; Sun, 21 May 2023 15:27:33 -0700 (PDT) In-Reply-To: Received-SPF: pass client-ip=2a00:1450:4864:20::334; envelope-from=dinnyesd@gmail.com; helo=mail-wm1-x334.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:21834 Archived-At: --000000000000175be605fc3ba914 Content-Type: text/plain; charset="UTF-8" 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 > > > --000000000000175be605fc3ba914 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi everyone,

It seems this h= asn't garnered much attention.

Nevertheles= s, meanwhile I've realised the code had a few bugs, in particular aroun= d the backwards-compatibility flag.

I've done = a major rewrite:
  • fixed couple of bugs I found in the orig= inal implementation
  • made implementation more robust, simpler, and b= ackwards-compatible:
    • added new expect-chars macro to support the new non-backwards-compat= ible matcher procedure signature.
    • removed need for expect-pass-char? backwards-compatibility feat= ure 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 <dinnyesd@gmail.com> wrote:
Hi everyone,<= /div>

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

The procedural expect macro has a ser= ious 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 i= dentifier bound to such a procedure, but instead an expression which create= s such a procedure (aka "factory"), then the expression will be e= valuated anew for each character read from expect-port. This is a serious p= roblem: when constructing that procedure has significant computational cost= s, or side effects (eg. opening a port to a new log file), as it can be see= n in my code here. (This is a branch of so= me of my KVM-based E2E testing code, which uses the original expect macro i= mplementation, just to demonstrate the problem).

I= had written a workaround for this problem, by binding the evaluation of th= e test expression outside the expansion of the expect macro (as it can be s= een here). The problem with this was, that it d= oesn't support the full capabilities of the original expect macro, and = debilitates it by only allowing for a single conditional branch. Also, I ha= ven't verified, but I suspicious it would possibly even break the behav= iour for the =3D> 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-ca= se. I've ran into some issues, and my implementation didn't = expand as I expected. After some discussion on IRC, we've conclu= ded 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 complet= ely to be hygienic instead.

I've eventually co= mpleted the rewrite using syntax-case here, and woul= d be happy to contribute it back to the ice-9 module!
I would poi= nt 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 (ic= e-9 expect). Switching to the new implementation was simple, and works flaw= lessly!
  2. It is backwards compatible! I've gone extra lengths= to make sure the code would be a backwards compatible, a drop-in replaceme= nt.
  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 c= haracter. There is multiple reasons this is superior to passing just a bool= ean eof? flag argument:
      1. the same logic can be implement= ed as with the eof? boolean, just by negating the condition
      2. pas= sing 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


--000000000000175be605fc3ba914--