Re: Guile: What's wrong with this?

[Mark H Weaver]

Mike Gran <address@hidden> writes:

>> From: Mark H Weaver <address@hidden>
>> No, `define' does not copy an object, it merely makes a new reference to
>> an existing object.  This is also true in C for that matter, so this is
>> behavior is quite mainstream.  For example, the following program dies
>> with SIGSEGV on most modern systems, including GNU/Linux:
>> 
>>   int
>>   main()
>>   {
>>     char *y = "hello";
>>     y[0] = 'a';
>>     return 0;
>>   }
>
>  
> True, but the following also is quite mainstream
> int main()
> {
>   char y[6] = "hello";
>   y[0] = 'a';
>   return 0;
> }
>  
> C provides a way to create and initialize a mutable string.

Scheme and Guile provide ways to do that too, but that's _never_ what
`define' has done.

>> Scheme and Guile are the same as C in this respect.  Earlier versions of
>> Guile didn't make a copy of the string in this case either, but it
>> lacked the mechanism to detect this error, and allowed you to modify the
>> string literal in the program text itself, which is a _very_ bad idea.
>
> It all depends on your mental model.  Your saying that (define y "hello")
> attaches "hello" to y, and since "hello" is a immutable, the string y
> contains must be immutable.  This is an argument based on purity, not
> utility.

If we were designing a new language, then it would at least be pertinent
to argue this point.  However, this is the way `define' has _always_
worked in every variant of Scheme, and the same is true of the analogous
`set' in Lisp from the very beginning.

> If you follow that logic, then Guile is left without any shorthand
> to create and initialize a mutable string other than
>  
> (define y (substring "hello" 0))
> or 
> (define y (string-copy "hello"))

Guile provides all the machinery you need to define shorthand syntax if
you like, e.g:

  (define-syntax-rule (define-string v s) (define v (string-copy s)))

For that matter, you could also do something like this:

  (define-syntax define
    (lambda (x)
      (with-syntax ((orig-define #'(@ (guile) define)))
        (syntax-case x ()
          ((_ (proc arg ...) e0 e1 ...)
           #'(orig-define proc (lambda (arg ...) e0 e1 ...)))
          ((_ v e)
           (identifier? #'v)
           (if (string? (syntax->datum #'e))
               #'(orig-define v (string-copy e))
               #'(orig-define v e)))))))

This will change `define' (in the module where it's defined) to
automatically copy a bare string literal on the right side.  Note that
this check is done at compile-time, so it can't look at the dynamic type
of an expression.

If that's not good enough and you're willing to take the efficiency hit
at runtime for _every_ use of `define', you could change `define' to
wrap the right-hand expression within a procedure call to check for
read-only strings:

  (define (copy-if-string x)
    (if (string? x)
        (string-copy x)
        x))
  
  (define-syntax define
    (lambda (x)
      (with-syntax ((orig-define #'(@ (guile) define)))
        (syntax-case x ()
          ((_ (proc arg ...) e0 e1 ...)
           #'(orig-define proc (lambda (arg ...) e0 e1 ...)))
          ((_ v e)
           #'(orig-define v (copy-if-string e)))))))

Scheme's nice handling of hygiene should make redefining `define' within
your own modules (including (guile-user)) harmless.  If it doesn't,
that's a bug and we'd like to hear about it.

> It was wrong to change this without deprecating it first.

The only change here was to add the machinery to detect an error that
was _always_ an error.  It _never_ did what you say that it should do.

What it did before was fail to detect that you were changing the string
constant in the program text itself.  The Guile 1.8 example I gave in my
last email in this thread demonstrates that.

To make that point even clearer, I'll post the full copy of the error
message Guile 1.8 gave when my loop ran past the end of the string:

  guile> (let loop ((i 0))
           (define y "hello")
           (display y)
           (newline)
           (string-set! y i #\a)
           (loop (1+ i)))
  hello
  aello
  aallo
  aaalo
  aaaao
  aaaaa
  
  Backtrace:
  In standard input:
     2: 0* [loop 0]
  In unknown file:
     ?: 1  (letrec ((y "aaaaa")) (display y) ...)
     ...
     ?: 2  (letrec ((y "aaaaa")) (display y) ...)
  In standard input:
     2: 3* [string-set! "aaaaa" {5} #\a]
  
  standard input:2:60: In procedure string-set! in expression (string-set! y i 
...):
  standard input:2:60: Value out of range 0 to 4: 5
  ABORT: (out-of-range)
  guile> 

Take a look at the backtrace, where it helpfully shows you an excerpt of
the source code (admittedly after some transformation).  See how the
source code itself has been modified?  This is what Bruce's code does.
It was _always_ a serious error in the code, even if it went undetected
in earlier versions of Guile.

     Mark