Hi!
2008/9/22 Ludovic Courtès <[EMAIL PROTECTED]>:
> Hi,
>
> "Bill Schottstaedt" <[EMAIL PROTECTED]> 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
