[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Use a fluid for the list of the reader's "hash procedures"

From: Ludovic Courtès
Subject: Re: [PATCH] Use a fluid for the list of the reader's "hash procedures"
Date: Wed, 03 Nov 2010 00:18:38 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux)

Hi Andreas!

Andreas Rottmann <address@hidden> writes:

> address@hidden (Ludovic Courtès) writes:
>> Hi!
>> Thanks for the patch.
>> Andreas Rottmann <address@hidden> writes:
>>> This is a prerequisite for the patch implementing SRFI-38, which I'll
>>> post next.
>>> From: Andreas Rottmann <address@hidden>
>>> Subject: Use a fluid for the list of the reader's "hash procedures"
>>> This allows customizing the reader behavior for a dynamic extent more 
>>> easily.
>>> * libguile/read.c (scm_read_hash_procedures): Renamed to
>>>   `scm_i_read_hash_procedures'.
>>>   (scm_i_read_hash_procedures_ref, scm_i_read_hash_procedures_set_x):
>>>   New (internal) accessor functions for the fluid.
>>>   (scm_read_hash_extend, scm_get_hash_procedure): Use these accessor
>>>   functions.
>>>   (scm_init_read): Create the fluid, named `%read-hash-procedures' instead 
>>> of 
>>>   the previous plain list `read-hash-procedures'.
>> Hmm that’s an incompatible change.  ‘scm_read_hash_procedures’ and
>> ‘read-hash-procedures’ aren’t documented, but they’re public.  
> The latter indeed was public -- I've added a compatibility shim,
> similiar to the one you suggested.

Great, applied!

> `scm_read_hash_procedures' was "static", so there should be no issue
> there.


> I kind-of assumed that undocumented functionality would not be held
> accountable to the normal deprecation rules.

Well, in some cases, we can’t ignore that undocumented functionality is
widely used; in other cases, we find no evidence that it was used.  So
undocumented functionality is really considered on a case-by-case basis.

> Now that we deprecate the functionality (and hence implicitly
> acknowledge that it might still be in use), I think it might make
> sense to either:
> - Make clear that `%read-hash-procedures' is "internal" -- the name kind
>   of implies that, but there are quite a few procedures that start with
>   `%' and are documented in the manual, so I cannot deduce what needs to
>   be done for a Scheme binding to be marked as internal, at least not
>   when it is defined in C and has to be accessible to the root module,
>   and hence is visible in the `(guile-user) module'
> - Document `%read-hash-procedures'.
> - Mark `%read-hash-procedures' as internal, but offer a documented way
>   to install a new fluid for a dynamic extent. E.g.:
> (define (with-read-hash-extensions extensions thunk)
>   (with-fluid* %read-hash-procedures (fluid-ref %read-hash-procedures)
>     (lambda ()
>       (for-each (lambda (extension)
>                    (read-hash-extend (car extension) (cdr extension)))
>                 extensions)
>       (thunk))))

Documenting ‘%read-hash-procedures’ is probably the easiest.  It exposes
implementation details, but OTOH, these details are unlikely to change
(there’s little incentive to change this.)

OTOOH (which one? :-)), ‘read-hash-extend’ exists precisely to hide
those details, which would warrant ‘with-read-hash-extensions’ as you


>> The latter can probably be kept compatible like this:
>>   (begin-deprecated
>>     (define %deprecated-read-hash-procedures
>>       (make-procedure-with-setter
>>         (lambda ()  (fluid-ref %read-hash-procedures))
>>         (lambda (v) (fluid-set! %read-hash-procedures v))))
>>     (define-syntax read-hash-procedures
>>       (lambda (s)
>>         (syntax-case s ()
>>           (_ (%deprecated-read-hash-procedures))))))
> Neat trick, I didn't know about `make-procedure-with-setter'.  My patch
> now contains equivalent functionality, but only relying on syntax-case,
> not `make-procedure-with-setter' -- it turned out you have to use
> `make-variable-transformer' or `identifier-syntax' if you want an
> identifier macro to work within a `set!' form.

Oh, nice.  I was unaware of the full power of ‘identifier-syntax’.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]