Precedence for reader extensions

Precedence for reader extensions

[Mikael Djurfeldt]

I'm working on an mit-scheme compatibility module (compat mit-scheme)
enabling Guile to read a (so far) subset of mit-scheme code.

Now I have the problem that mit-scheme has the two constants
#!optional and #!rest (mit-scheme extensions to the scheme standard).

I thought that I could support this using %read-hash-procedures but
discovered that there are three "precedence levels":

1. Most predefine hash syntax like #(, #! etc.
2. %read-hash-procedures
3. #|

This means that I can't add new syntax for #!.

I propose to simplify this to only two levels:

1. %read-hash-procedures
2. predefined syntax

This would be conceptually simpler and more flexible.  It could also
be used to support mit-scheme read syntax.

If we do not implement this change, I need to use Ludovic's (nice)
guile-reader.  But that package contains C code meaning that (compat
mit-scheme) can't be distributed using guilehall.  In any case, I
think it would be good to be able to support other Scheme's read
syntax using the standard reader.

If anyone is afraid about the effect this would have on reader
performance, it is possible to compile %read-hash-procedures to a
table of flags indicating exceptions.

Opinions?

Best regards,
Mikael

Re: Precedence for reader extensions

[Mikael Djurfeldt]

On Mon, Feb 18, 2013 at 10:05 PM, Mikael Djurfeldt <mikael@djurfeldt.com> wrote:
> guilehall

guildhall

(I write too fast)

Re: Precedence for reader extensions

[Mikael Djurfeldt]

On Mon, Feb 18, 2013 at 10:05 PM, Mikael Djurfeldt <mikael@djurfeldt.com> wrote:
> If anyone is afraid about the effect this would have on reader
> performance, it is possible to compile %read-hash-procedures to a
> table of flags indicating exceptions.

But, given the current interface to reader extensions in the form of
an alist in a fluid, such a table would have to be compiled at each
entry to "read"...

Re: Precedence for reader extensions

[Mikael Djurfeldt]

(Sorry for thinking publicly.)

The reason why I don't simply use guile-reader but start bugging you
about it is that it feels silly that Guile, which was originally
supposed to be able to support different languages, can't even support
the read syntax of a sibling scheme interpreter.  It is somewhat
inflexible.

What about including Ludovic's guile-reader as a library in the main
guile distribution?

Re: Precedence for reader extensions

[Mark H Weaver]

Hi Mikael,

Mikael Djurfeldt <mikael@djurfeldt.com> writes:
> I'm working on an mit-scheme compatibility module (compat mit-scheme)
> enabling Guile to read a (so far) subset of mit-scheme code.
>
> Now I have the problem that mit-scheme has the two constants
> #!optional and #!rest (mit-scheme extensions to the scheme standard).
>
> I thought that I could support this using %read-hash-procedures but
> discovered that there are three "precedence levels":
>
> 1. Most predefine hash syntax like #(, #! etc.
> 2. %read-hash-procedures
> 3. #|
>
> This means that I can't add new syntax for #!.
>
> I propose to simplify this to only two levels:
>
> 1. %read-hash-procedures
> 2. predefined syntax

I don't think this would be sufficient.  The problem is that tokens of
the form "#!<symbol><delimiter>" have become standardized.  To name a
few examples, both R6RS and R7RS define the reader directives
#!fold-case and #!no-fold-case, R6RS has #!r6rs, and SRFI-105 has
#!curly-infix.  Guile also has #! ... !# block comments to help with the
handling of executable scripts.

One idea would be to provide a way to register new handlers for tokens
of the form "#!<symbol><delimiter>".  However, we'd probably want to
discourage its use, because every time you add a new token, you
potentially turn existing #! ... !# block comments into lexical errors.

Thanks for working on scmutils for Guile!
I've been wanting this for a while :)

      Mark

Re: Precedence for reader extensions

[Mikael Djurfeldt]

[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]

On Tue, Feb 19, 2013 at 12:33 AM, Mark H Weaver <mhw@netris.org> wrote:
> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>> I propose to simplify this to only two levels:
>>
>> 1. %read-hash-procedures
>> 2. predefined syntax

It turns out that the change I propose above was already implemented
in read.c.  The effect just wasn't visible due to a bug in flush_ws
which caused all #! to be erroneously removed if they exist as the
outermost expression.

In the attached diff, I've fixed the flush_ws bug and cleaned up some
garbage code in scm_read_sharp which was unreachable.

Can I push this into the repository?

> I don't think this would be sufficient.  The problem is that tokens of
> the form "#!<symbol><delimiter>" have become standardized.  To name a
> few examples, both R6RS and R7RS define the reader directives
> #!fold-case and #!no-fold-case, R6RS has #!r6rs, and SRFI-105 has
> #!curly-infix.  Guile also has #! ... !# block comments to help with the
> handling of executable scripts.

In what sense is it not sufficient?  In any case: The present diff
doesn't remove any functionality or make performance worse.  It only
removes some inconsistent behavior.  At the same time it allows
support for mit-scheme #!optional and #!rest.

Best regards,
Mikael D.

[-- Attachment #2: reader-fix.diff --]
[-- Type: application/octet-stream, Size: 2148 bytes --]

diff --git a/libguile/read.c b/libguile/read.c
index 7c003b4..71eae32 100644
--- a/libguile/read.c
+++ b/libguile/read.c
@@ -356,7 +356,13 @@ flush_ws (SCM port, scm_t_read_opts *opts, const char *eoferr)
 	break;
 
       case '#':
-	switch (c = scm_getc_unlocked (port))
+	c = scm_getc_unlocked (port);
+	if (scm_is_true (scm_procedure_p (scm_get_hash_procedure (c))))
+	  {
+	    scm_ungetc_unlocked (c, port);
+	    return '#';
+	  }
+	switch (c)
 	  {
 	  case EOF:
 	    eoferr = "read_sharp";
@@ -368,12 +374,8 @@ flush_ws (SCM port, scm_t_read_opts *opts, const char *eoferr)
 	    scm_read_commented_expression (c, port, opts);
 	    break;
 	  case '|':
-	    if (scm_is_false (scm_get_hash_procedure (c)))
-	      {
-		scm_read_r6rs_block_comment (c, port);
-		break;
-	      }
-	    /* fall through */
+	    scm_read_r6rs_block_comment (c, port);
+	    break;
 	  default:
 	    scm_ungetc_unlocked (c, port);
 	    return '#';
@@ -1626,6 +1628,8 @@ scm_read_sharp (scm_t_wchar chr, SCM port, scm_t_read_opts *opts,
       return (scm_read_extended_symbol (chr, port));
     case '!':
       return (scm_read_shebang (chr, port, opts));
+    case '|':
+      return scm_read_r6rs_block_comment (chr, port);
     case ';':
       return (scm_read_commented_expression (chr, port, opts));
     case '`':
@@ -1635,23 +1639,8 @@ scm_read_sharp (scm_t_wchar chr, SCM port, scm_t_read_opts *opts,
     case 'n':
       return (scm_read_nil (chr, port, opts));
     default:
-      result = scm_read_sharp_extension (chr, port, opts);
-      if (scm_is_eq (result, SCM_UNSPECIFIED))
-	{
-	  /* To remain compatible with 1.8 and earlier, the following
-	     characters have lower precedence than `read-hash-extend'
-	     characters.  */
-	  switch (chr)
-	    {
-	    case '|':
-	      return scm_read_r6rs_block_comment (chr, port);
-	    default:
-	      scm_i_input_error (FUNC_NAME, port, "Unknown # object: ~S",
-				 scm_list_1 (SCM_MAKE_CHAR (chr)));
-	    }
-	}
-      else
-	return result;
+      scm_i_input_error (FUNC_NAME, port, "Unknown # object: ~S",
+			 scm_list_1 (SCM_MAKE_CHAR (chr)));
     }
 
   return SCM_UNSPECIFIED;

Re: Precedence for reader extensions

[Mark H Weaver]

Mikael Djurfeldt <mikael@djurfeldt.com> writes:
> On Tue, Feb 19, 2013 at 12:33 AM, Mark H Weaver <mhw@netris.org> wrote:
>> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>>> I propose to simplify this to only two levels:
>>>
>>> 1. %read-hash-procedures
>>> 2. predefined syntax
>
> It turns out that the change I propose above was already implemented
> in read.c.  The effect just wasn't visible due to a bug in flush_ws
> which caused all #! to be erroneously removed if they exist as the
> outermost expression.

I'm not sure that I consider this a bug.  All of the tokens that
flush_ws removes can appear anywhere that whitespace is allowed, and are
considered whitespace to the caller (although reader directives may
modify the per-port reader options as a side-effect).

  #;<expr>     (sexp-comments)
  #!fold-case  (reader directives)
  #! ... !#    (shebang block comments)
  #| ... |#    (r6rs block comments)

> In the attached diff, I've fixed the flush_ws bug and cleaned up some
> garbage code in scm_read_sharp which was unreachable.
>
> Can I push this into the repository?

I'm uncomfortable with globally overriding standard read syntax.  In a
large scheme system such as Guile, there are many modules that use
'read' and expect it to act in accordance with standard lexical
conventions.

Therefore, I'd prefer to limit 'read-hash-extend' to adding new syntax
that would otherwise have been considered an error.  If you're going to
override standard read syntax, then I think it should only be done on a
per-port basis.

Therefore, I'd prefer a precedence closer to this:

  1. (possibly) per-port variant of %read-hash-procedures
  2. predefined syntax
  3. %read-hash-procedures

>> I don't think this would be sufficient.  The problem is that tokens of
>> the form "#!<symbol><delimiter>" have become standardized.  To name a
>> few examples, both R6RS and R7RS define the reader directives
>> #!fold-case and #!no-fold-case, R6RS has #!r6rs, and SRFI-105 has
>> #!curly-infix.  Guile also has #! ... !# block comments to help with the
>> handling of executable scripts.
>
> In what sense is it not sufficient?  In any case: The present diff
> doesn't remove any functionality or make performance worse.  It only
> removes some inconsistent behavior.  At the same time it allows
> support for mit-scheme #!optional and #!rest.

The problem with this approach is that it does not compose.  You want to
add #!optional and #!rest.  R6RS added #!r6rs, #!fold-case, and
#!no-fold-case.  SRFI-105 added #!curly-infix.  But there can be only
one read-hash-procedure for #!, and it's global to the entire system.

That's why I suggested a way to add new tokens of the form
"#!<symbol><delimiter>".  That way, you could add handlers for #!rest
and #!optional without interfering with the other #!<symbol><delimiter>
tokens.

What do you think?

    Regards,
      Mark

Re: Precedence for reader extensions

[Mikael Djurfeldt]

[-- Attachment #1: Type: text/plain, Size: 2452 bytes --]

On Tue, Feb 19, 2013 at 4:41 PM, Mark H Weaver <mhw@netris.org> wrote:
> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>> On Tue, Feb 19, 2013 at 12:33 AM, Mark H Weaver <mhw@netris.org> wrote:
>>> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>>>> I propose to simplify this to only two levels:
>>>>
>>>> 1. %read-hash-procedures
>>>> 2. predefined syntax
>>
>> It turns out that the change I propose above was already implemented
>> in read.c.  The effect just wasn't visible due to a bug in flush_ws
>> which caused all #! to be erroneously removed if they exist as the
>> outermost expression.
>
> I'm not sure that I consider this a bug.

In this reply I've attached a file mit-reader-scm which installs a
hash-read-procedure for #\!.

What I wanted to say above is that scm_read_sharp (in HEAD) is
implemented with the priorities I list above while flush_ws is
implemented with other priorities.  Here's a demo of the consequences
of this bug:

scheme@(guile-user)> (load "mit-reader.scm")
scheme@(guile-user)> (quote #!optional)
... !# hi)
$1 = hi
scheme@(guile-user)> '#!optional
$2 = #:optional

> [...]
> I'm uncomfortable with globally overriding standard read syntax.  In a
> large scheme system such as Guile, there are many modules that use
> 'read' and expect it to act in accordance with standard lexical
> conventions.

Well, in the mit-scheme compatibility module, my intention was to use
dynamic-wind to modify #!-syntax while loading mit-scheme-specific
files.  Note that %read-hash-procedures is a fluid, so this will be
absolutely local and won't leak out in any way to the rest of the
system.

> The problem with this approach is that it does not compose.

Let's now patch guile according to the diff I sent... there!

scheme@(guile-user)> (load "mit-reader.scm")
scheme@(guile-user)> (quote #!optional)
$1 = #:optional
scheme@(guile-user)> '#!optional
$2 = #:optional
scheme@(guile-user)> (quote #!hi!# #!optional)
$3 = #:optional

My take on this is:

* The %read-hash-procedures API is not pretty

* The suggested change doesn't make things prettier

* The suggested change *does* make things conceptually simpler and
more flexible (= you can always override hash syntax if you want;
compared to the current: you can override #| but not other hash
syntax)

* The suggested change fixes a bug

* The suggested change does compose and different syntax can be
confined to a module by using dynamic-wind

Best regards,
Mikael

[-- Attachment #2: mit-reader.scm --]
[-- Type: application/octet-stream, Size: 774 bytes --]

(define (%read-mit-constants c port)
  (define (try-read string)
    (let ((length (string-length string)))
      (let loop ((i 0))
	(cond ((= i length) (symbol->keyword (string->symbol string)))
	      ((not (char=? (peek-char port) (string-ref string i)))
	       (unread-string (substring string 0 i) port)
	       *unspecified*)
	      (else (read-char port)
		    (loop (+ 1 i)))))))
  (case (peek-char port)
    ((#\o) (try-read "optional"))
    ((#\r) (try-read "rest"))))

(define (install-hash-procedure! c proc)
  (let ((als (fluid-ref %read-hash-procedures )))
    (cond ((assv c als)
	   => (lambda (p)
		(set-cdr! p proc)))
	  (else
	   (fluid-set! %read-hash-procedures
		       (cons (cons c proc) als))))))

(install-hash-procedure! #\! %read-mit-constants)

Re: Precedence for reader extensions

[Mikael Djurfeldt]

On Tue, Feb 19, 2013 at 5:42 PM, Mikael Djurfeldt <mikael@djurfeldt.com> wrote:
> * The suggested change does compose

What I meant here is that it does compose with the built-in syntax.

Of course, the %read-hash-procedures API by itself doesn't
automatically compose if multiple user-defined modules use it to
introduce new syntax.  (If these modules take care to preserve
previously installed procedures, it can compose.)

The API you suggest would compose much easier, but to me it feels like
just another specialized solution.  What we would really need is
something like Ludovic's guile-reader.

But I won't be stubborn regarding this.  If someone else wants to
implement another way of supporting #!optional and #!rest that is fine
by me.  I regard my diff simply as a bug fix and cleanup (removing
unreachable code).

Re: Precedence for reader extensions

[Ludovic Courtès]

Hi!

Mikael Djurfeldt <mikael@djurfeldt.com> skribis:

> The API you suggest would compose much easier, but to me it feels like
> just another specialized solution.  What we would really need is
> something like Ludovic's guile-reader.

In general, I think it should be easy to create new readers that derive
from the standard syntax without having to write them from scratch.

However, in hindsight, I’m not sure Guile-Reader’s API is the right
approach.  It’s an improvement, because it addresses this need; but its
API is not ideal: “token readers” with different delimiter syntax don’t
compose well, for instance.

> But I won't be stubborn regarding this.  If someone else wants to
> implement another way of supporting #!optional and #!rest that is fine
> by me.  I regard my diff simply as a bug fix and cleanup (removing
> unreachable code).

This is basically DSSSL keyword syntax.  What about adding a new keyword
style to read.c?  Sounds like the easiest solution for this particular
problem.

Ludo’.

Re: Precedence for reader extensions

[Mark H Weaver]

Mikael Djurfeldt <mikael@djurfeldt.com> writes:
> The API you suggest would compose much easier, but to me it feels like
> just another specialized solution.  What we would really need is
> something like Ludovic's guile-reader.

I agree that we should ideally have a much more general way of defining
customized readers.  In the meantime, my primary concern is to find a
solution to your problem without committing us to supporting an overly
general mechanism that fails to provide basic guarantees to other users
of 'read'.

As you pointed out, the current code *almost* supports overriding
standard syntax for things like "#!".  However, it has been broken for
a long time.  The same bug is in Guile 1.8, and I haven't seen anyone
complaining about it.  Therefore, I'm more inclined to remove this
broken functionality than to fix it.

Mikael Djurfeldt <mikael@djurfeldt.com> writes:
> But I won't be stubborn regarding this.  If someone else wants to
> implement another way of supporting #!optional and #!rest that is fine
> by me.

Thanks.  I hope to cook up a patch in the next few days.

Ludovic Courtès <ludo@gnu.org> writes:
> This is basically DSSSL keyword syntax.  What about adding a new keyword
> style to read.c?  Sounds like the easiest solution for this particular
> problem.

This is a tempting solution, but I see a problem with this proposal:
We'd have to make exceptions for things like #!fold-case and
#!curly-infix, as well as for things like #!/usr/bin/guile.  Also, it
could potentially turn existing scsh-style block comments into syntax
errors.

Ludovic Courtès <ludo@gnu.org> writes:
> In general, I think it should be easy to create new readers that derive
> from the standard syntax without having to write them from scratch.
> 
> However, in hindsight, I’m not sure Guile-Reader’s API is the right
> approach.  It’s an improvement, because it addresses this need; but its
> API is not ideal: “token readers” with different delimiter syntax don’t
> compose well, for instance.

I'd be very interested to hear your current thoughts on what a better
API should look like.

     Regards,
       Mark

Re: Precedence for reader extensions

[Mikael Djurfeldt]

Thanks, Mark.  This all sounds very sensible to me, and I will
continue working on the scmutils port while waiting for your patch.

On Fri, Feb 22, 2013 at 3:52 AM, Mark H Weaver <mhw@netris.org> wrote:
> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>> The API you suggest would compose much easier, but to me it feels like
>> just another specialized solution.  What we would really need is
>> something like Ludovic's guile-reader.
>
> I agree that we should ideally have a much more general way of defining
> customized readers.  In the meantime, my primary concern is to find a
> solution to your problem without committing us to supporting an overly
> general mechanism that fails to provide basic guarantees to other users
> of 'read'.
>
> As you pointed out, the current code *almost* supports overriding
> standard syntax for things like "#!".  However, it has been broken for
> a long time.  The same bug is in Guile 1.8, and I haven't seen anyone
> complaining about it.  Therefore, I'm more inclined to remove this
> broken functionality than to fix it.
>
> Mikael Djurfeldt <mikael@djurfeldt.com> writes:
>> But I won't be stubborn regarding this.  If someone else wants to
>> implement another way of supporting #!optional and #!rest that is fine
>> by me.
>
> Thanks.  I hope to cook up a patch in the next few days.
>
> Ludovic Courtès <ludo@gnu.org> writes:
>> This is basically DSSSL keyword syntax.  What about adding a new keyword
>> style to read.c?  Sounds like the easiest solution for this particular
>> problem.
>
> This is a tempting solution, but I see a problem with this proposal:
> We'd have to make exceptions for things like #!fold-case and
> #!curly-infix, as well as for things like #!/usr/bin/guile.  Also, it
> could potentially turn existing scsh-style block comments into syntax
> errors.
>
> Ludovic Courtès <ludo@gnu.org> writes:
>> In general, I think it should be easy to create new readers that derive
>> from the standard syntax without having to write them from scratch.
>>
>> However, in hindsight, I’m not sure Guile-Reader’s API is the right
>> approach.  It’s an improvement, because it addresses this need; but its
>> API is not ideal: “token readers” with different delimiter syntax don’t
>> compose well, for instance.
>
> I'd be very interested to hear your current thoughts on what a better
> API should look like.
>
>      Regards,
>        Mark

Re: Precedence for reader extensions

[Ludovic Courtès]

Mikael Djurfeldt <mikael@djurfeldt.com> skribis:

> Thanks, Mark.  This all sounds very sensible to me, and I will
> continue working on the scmutils port while waiting for your patch.

BTW, in the meantime you can always use Guile-Reader, which supports
DSSSL keyword syntax.

Ludo’.

Re: Precedence for reader extensions

[Ludovic Courtès]

Mark H Weaver <mhw@netris.org> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>> This is basically DSSSL keyword syntax.  What about adding a new keyword
>> style to read.c?  Sounds like the easiest solution for this particular
>> problem.
>
> This is a tempting solution, but I see a problem with this proposal:
> We'd have to make exceptions for things like #!fold-case and
> #!curly-infix, as well as for things like #!/usr/bin/guile.  Also, it
> could potentially turn existing scsh-style block comments into syntax
> errors.

The DSSSL option could be documented as incompatible with those other
reader options, and perhaps an error could be raised when an attempt to
use incompatible options is made.  WDYT?

Admittedly, this would increase reader complexity.

> Ludovic Courtès <ludo@gnu.org> writes:
>> In general, I think it should be easy to create new readers that derive
>> from the standard syntax without having to write them from scratch.
>> 
>> However, in hindsight, I’m not sure Guile-Reader’s API is the right
>> approach.  It’s an improvement, because it addresses this need; but its
>> API is not ideal: “token readers” with different delimiter syntax don’t
>> compose well, for instance.
>
> I'd be very interested to hear your current thoughts on what a better
> API should look like.

Probably parser combinators, like
<http://planet.racket-lang.org/display.ss?package=combinator-parser.plt&owner=plt>.

In Guile we have the extra restriction that we need to keep ‘read’ in C
for bootstrapping, so that would probably mean having two reader
implementations.

Ludo’.

Re: Precedence for reader extensions

[Mikael Djurfeldt]

On Tue, Feb 19, 2013 at 5:42 PM, Mikael Djurfeldt <mikael@djurfeldt.com> wrote:
> * The suggested change *does* make things conceptually simpler and
> more flexible (= you can always override hash syntax if you want;
> compared to the current: you can override #| but not other hash
> syntax)

Just to try to be clear:

What I write above is not strictly true.  The current Guile *already*
allows you to override standard syntax, even without my changes.  What
my changes do is to cleanup the old mechanism so that it doesn't fail
when whitespace is involved.

An example of how it currently fails is that you *can* override when
you spell quote using single quote ("'OBJECT") since no whitespace is
involved while you cannot override when you spell quote using the
symbol quote ("(quote OBJECT)") since there's whitespace to be
swallowed before the OBJECT.

I do respect the attitude that the user shouldn't be able to override
standard syntax, even though I don't think it matters much given the
state of the current mess.  But I think you agree that we either need
to apply my fix (making the current overriding mechanism useful) or
fix scm_read_sharp so that it conforms with the behavior of flush_ws.