Re: Bug in shared substrings?

[Charbel Jacquin <cjacquin@club-internet.fr>]

Redford, John wrote:
> 
> This bit of code seems to demonstrate a bug with shared substrings. Can
> someone confirm this, or else point out my mistake?
> 
> Normally I'd spend some time to investigate this more, but with a
> release being talked about soon, I wanted to get this out rapidly.
> 
> Thanks.
> 
> (define (no-bug text)
>   (let ((p (string-index text #\X)))
>     (if p
>         (begin (string-set! text p #\_)
>                (no-bug text)))))
> 
> (define (bug text)
>   (let ((p (string-index text #\X)))
>     (if p
>         (begin (string-set! text p #\_)
>                (bug (make-shared-substring text (+ p 1)))))))
> 
> (write-line "Correct")
> (define x "111X222X333X444X555")
> (write-line x)
> (no-bug x)
> (write-line x)
> 
> (write-line "Buggy")
> (define x "111X222X333X444X555")
> (write-line x)
> (bug x)
> (write-line x)

The folloying patch to guile-core/libguile/strings.c seems to work.

--- strings.c.buggy     Tue Oct  6 04:24:09 1998
+++ strings.c   Tue Oct  6 04:24:36 1998
@@ -286,7 +286,7 @@
   SCM_ASSERT (SCM_INUMP (k), k, SCM_ARG2, s_string_set_x);
   SCM_ASSERT (SCM_ICHRP (chr), chr, SCM_ARG3, s_string_set_x);
   SCM_ASSERT (SCM_INUM (k) < SCM_LENGTH (str) && SCM_INUM (k) >= 0, k,
SCM_OUTOFRANGE, s_string_set_x);
-  SCM_UCHARS (str)[SCM_INUM (k)] = SCM_ICHR (chr);
+  SCM_ROUCHARS (str)[SCM_INUM (k)] = SCM_ICHR (chr);
   return SCM_UNSPECIFIED;
 }

But according to (my copy of) the doc, a shared-substring should bee
immutable. So this patch may be better:
  * calling string-set! on a shared-substring now raises 
    a wrong-type-arg error.
  * calling substring-set! on a string is equivalent to calling
    string-set!.
  * calling substring-set! on a shared-substring modifies the underlying
    string.

--- strings.c.buggy     Tue Oct  6 04:24:09 1998
+++ strings.c   Tue Oct  6 04:35:59 1998
@@ -282,11 +282,29 @@
      SCM k;
      SCM chr;
 {
-  SCM_ASSERT (SCM_NIMP (str) && SCM_STRINGP (str), str, SCM_ARG1,
s_string_set_x);
+  SCM_ASSERT (SCM_NIMP (str) && SCM_STRINGP (str) &&
SCM_TYP7(str)!=scm_tc7_substring, 
+             str, SCM_ARG1, s_string_set_x);
   SCM_ASSERT (SCM_INUMP (k), k, SCM_ARG2, s_string_set_x);
   SCM_ASSERT (SCM_ICHRP (chr), chr, SCM_ARG3, s_string_set_x);
   SCM_ASSERT (SCM_INUM (k) < SCM_LENGTH (str) && SCM_INUM (k) >= 0, k,
SCM_OUTOFRANGE, s_string_set_x);
   SCM_UCHARS (str)[SCM_INUM (k)] = SCM_ICHR (chr);
+  return SCM_UNSPECIFIED;
+}
+
+
+SCM_PROC(s_substring_set_x, "substring-set!", 3, 0, 0,
scm_substring_set_x);
+
+SCM
+scm_substring_set_x (str, k, chr)
+     SCM str;
+     SCM k;
+     SCM chr;
+{
+  SCM_ASSERT (SCM_NIMP (str) && SCM_STRINGP (str), str, SCM_ARG1,
s_substring_set_x);
+  SCM_ASSERT (SCM_INUMP (k), k, SCM_ARG2, s_substring_set_x);
+  SCM_ASSERT (SCM_ICHRP (chr), chr, SCM_ARG3, s_substring_set_x);
+  SCM_ASSERT (SCM_INUM (k) < SCM_LENGTH (str) && SCM_INUM (k) >= 0, k,
SCM_OUTOFRANGE, s_substring_set_x);
+  SCM_ROUCHARS (str)[SCM_INUM (k)] = SCM_ICHR (chr);
   return SCM_UNSPECIFIED;
 }
 
Charbel Jacquin