Re: string-set! examples in r5rs.html

[Neil Jerram]

Hi!

2008/9/22 Ludovic Courtès <address@hidden>:
> Hi,
>
> "Bill Schottstaedt" <address@hidden> writes:
>
>> according to r5rs.html, these should signal an error, I believe:
>>
>> guile> (string-set! (symbol->string 'immutable)
>>              0
>>              #\?)
>> guile> (define (g) "***")
>> guile> (string-set! (g) 0 #\?)
>> guile> (g)
>> "?**"
>
> The attached patches against 1.8.x fix this.
>
> Neil: OK to apply?

Yes.  I have a few queries, which could lead to minor changes, but
nothing that important...

-(use-modules (ice-9 documentation))
+(define-module (test-suite test-symbols)
+  #:use-module (test-suite lib)
+  #:use-module (ice-9 documentation))

That's an interesting detail that I haven't noticed before.  (i.e. I
hadn't noticed that some of our test files have a define-module, and
some haven't.)  I've no objection to the define-module, but I'm
curious about why you added it.  If it makes an important difference,
it might be worth commenting.

+  if (SCM_UNLIKELY (STRING_LENGTH (str)) == 0)
+    /* We want the empty string to be `eq?' with the read-only empty
+       string.  */
+    return str;

Completely agree, but I wonder if we should be doing this optimization
in scm_i_substring_read_only() and scm_i_substring(), instead of here?
 Then we'd catch more cases.

> If yes, I would eventually take this opportunity to make
> `make-read-only-string' public.

Well, there is already scm_c_substring_read_only().  It feels like it
would be confusing to add another make-a-read-only-string API.  And if
your "" optimization was in scm_i_substring_read_only(),
scm_c_substring_read_only() would already be fine, wouldn't it?

Finally, while looking through related code, I noticed this in strings.c:

#if SCM_ENABLE_DEPRECATED

/* When these definitions are removed, it becomes reasonable to use
   read-only strings for string literals.  For that, change the reader
   to create string literals with scm_c_substring_read_only instead of
   with scm_c_substring_copy.
*/

...

Is that a concern?  I don't immediately see why these deprecated
functions are supposed to conflict with making literal strings
read-only, but the person who wrote that comment (probably Marius)
seemed to think there would be a conflict...

Regards,
      Neil