Hi!
Regarding SRFI-105, I’m skeptical about a couple of things.
First, $bracket-apply$, $nfx$, and $bracket-list$ need to be
user-defined, but implementations are allowed to provide a pre-defined
version of these. This sounds like an opportunity for incompatibilities
(which the document describes as a shortcoming of Guile’s infix module.)
It’s also unhygienic, in the sense that programs that need it would
typically have to start with a definition of $nfx$ & co., although these
identifiers never appear literally in the neoteric code.
Bracket lists (and $bracket-list$) are another opportunity for
incompatibilities, IMO. The SRFI reads:
several Scheme implementations follow the R6RS specification that
accepts [...] as a synonym for (...), GNU Kawa interprets [...] as the
redefinable constructor ($bracket-list$ ...)
But I fail to see why R6 syntax would influence SRFI-105 syntax.
That said...
Mark H Weaver <m...@netris.org> skribis:
> From 3345a52824c2ae0e6ffe64ec7e07609d1bc362ef Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <m...@netris.org>
> Date: Wed, 24 Oct 2012 14:50:16 -0400
> Subject: [PATCH 3/3] Implement SRFI-105 curly infix expressions.
>
> * libguile/private-options.h: Add SCM_CURLY_INFIX_P macro, and increment
> SCM_N_READ_OPTIONS.
>
> * libguile/read.c (sym_nfx, sym_bracket_list, sym_bracket_apply): New
> symbols.
> (scm_read_opts): Add curly-infix reader option.
> (scm_t_read_opts): Add curly_infix_p and neoteric_p fields.
> (init_read_options): Initialize new fields.
> (CHAR_IS_DELIMITER): Add '{', '}', '[', and ']' as delimiters if
> curly_infix_p is set.
>
> (set_port_square_brackets_p, set_port_curly_infix_p): New functions.
>
> (scm_read_expression_1): New internal static function, which contains
> the code that was previously in 'scm_read_expression'. Handle curly
> braces when curly_infix_p is set. If curly_infix_p is set and
> square_brackets_p is unset, follow the Kawa convention:
> [...] => ($bracket-list$ ...)
>
> (scm_read_expression): New function body to handle neoteric
> expressions where appropriate.
>
> (scm_read_shebang): Handle the new reader directives: '#!curly-infix'
> and the non-standard '#!curly-infix-and-bracket-lists'.
>
> (scm_read_sexp): Handle curly infix lists.
>
> * module/ice-9/boot-9.scm (%cond-expand-features): Add srfi-105 feature
> identifier.
>
> * doc/ref/srfi-modules.texi (SRFI-105): Add stub doc for SRFI-105.
>
> * doc/ref/api-evaluation.texi (Scheme Read): Add documentation for the
> 'curly-infix' read option, and the '#!curly-infix' and
> '#!curly-infix-and-bracket-lists' reader directives.
>
> * doc/ref/api-options.texi (Runtime Options): Add 'curly-infix' to the
> list of read options.
>
> * test-suite/Makefile.am: Add tests/srfi-105.test.
>
> * test-suite/tests/srfi-105.test: New file.
Besides, the patch is OK to apply for me, modulo the minor comments
below.
> +Guile also implements the following non-standard extension to SRFI-105:
> +if @code{curly-infix} is enabled but the @code{square-brackets} read
> +option is turned off, then lists within square brackets are read as
> +normal lists but with the special symbol @code{$bracket-list$} added to
> +the front. To enable this combination of read options within a file,
> +use the reader directive @code{#!curly-infix-and-bracket-lists}. For
> +example:
Do you think it would be possible, or even desirable, to be able to turn
off this extension?
> scm_t_option scm_read_opts[] = {
Can you move that brace on the next line?
> @@ -98,6 +105,8 @@ struct t_read_opts {
Ditto.
> + /* (len == 0) case is handled above */
> + if (len == 1)
> + /* Return directly to avoid re-annotating the element's source
> + location with the position of the outer brace. Also, it
> + might not be possible to annotate the element. */
> + return scm_car (ans); /* {e} => e */
> + else if (len == 2)
> + ; /* Leave the list unchanged: {e1 e2} => (e1 e2) */
> + else if (len >= 3 && (len & 1))
> + {
> + SCM op = scm_cadr (ans);
Can you add a comment above this line describing what case this
corresponds to? Like:
This is a longer list; check whether it contains mixed infix
operators, or if it’s an improper list.
> +scm_read_expression_1 (SCM port, scm_t_read_opts *opts)
What about calling it ‘read_inner_expression’, for instance?
> -#define READ_OPTIONS_NUM_BITS 14
> +#define READ_OPTIONS_NUM_BITS 16
I think I had missed that one. Can you add a comment above to describe
it?
> +(define-module (test-srfi-105)
> + #:use-module (test-suite lib)
> + #:use-module (srfi srfi-1))
> +
> +#!curly-infix
> +
> +(with-test-prefix "curly-infix"
> + (pass-if (equal? '{n <= 5} '(<= n 5)))
This is testing both the #! syntax and the actual infix parsing.
Something like ‘with-read-options’ from reader.test could be used, but
perhaps that’s an unnecessary complication.
> + ;;(pass-if (equal? '#1=f(#1#) '#1=(f #1#)))
Not implemented yet?
> + ;;(pass-if (equal? '#1={a + . #1#} '($nfx$ . #1=(a + . #1#))))
Same?
This sounds like a great test suite.
However, could you add a few tests regarding source location tracking,
by reading from a string port?
Thanks!
Ludo’.
