String beMutable SUnit bug/feature

In VW 7.4 the following test behaves rather old. The first time I run  
the test using it will pass. After that it will fail on the first  
assert. After the first run of the test on the second line of the  
method request be 'bat'. It will continue to fail until the method is  
recompiled. After one recompiles the method you get one run that  
passes. Does anyone else see this or am I blessed? What is going on  
here?

testMySanity
        | request |
        request := 'cat'  beMutable.
        self assert: request  = 'cat'.
        request at: 1 put: $b.
        self assert: request  = 'bat'.


Now if one changes the first line to either:

        request := 'cat' copy beMutable .

or

        request := 'cat' beMutable copy.

the test will always pass.
----
Roger Whitney              Department of Computer Science
[hidden email]        San Diego State University
http://www.eli.sdsu.edu/   San Diego, CA 92182-7720
(619) 583-1978
(619) 594-3535 (office)
(619) 594-6746 (fax)

From: Roger Whitney [mailto:[hidden email]]
That's the standard behaviour: a literal string in source compiles to a
literal string in the CompiledMethod. Anything you do to that literal
string when running the method will then affect it permanently in the
CompiledMethod, i.e. when you change it you are changing your compiled
code. The change doesn't affect the source code, so you don't see it in
the browser, but if you look at the compiled code of that method in an
inspector (e.g. with "Inspect" from the selector's popup menu), you will
see the change there.

That's precisely why literal strings in methods are immutable: changing
them is almost certainly not what the programmer intended, so the change
is prevented. When you sent #beMutable, you just took the safety off
:-).

What you want to do is just
  request := 'cat' copy.
The copy will already be mutable by default - sending it #beMutable is
harmless but unnecessary.

HTH,
Steve

Roger

When the method is compiled the lines
        request := 'cat'  beMutable.
        self assert: request  = 'cat'.
result in two literals in the literal array.

When run the line
        request := 'cat'  beMutable.
associates the content of the first literal with the temp var request
(and makes it mutable). The line
        self assert: request  = 'cat'.
Compares the contents of the second literal with the first. Then the
line
        request at: 1 put: $b.
changes the value of the first literal.

Hence when the method is run again the value of the first literal is
'bat' and the comparison with the second fails.

This is one of the classic Smalltalk bugs and is one of the often
mentioned reasons for having immutable literals.

Regards Joerg

-----Original Message-----
From: Roger Whitney [mailto:[hidden email]]
Sent: Wednesday, February 01, 2006 12:34 PM
To: vwnc List
Subject: String beMutable SUnit bug/feature

In VW 7.4 the following test behaves rather old. The first time I run  
the test using it will pass. After that it will fail on the first  
assert. After the first run of the test on the second line of the  
method request be 'bat'. It will continue to fail until the method is  
recompiled. After one recompiles the method you get one run that  
passes. Does anyone else see this or am I blessed? What is going on  
here?

testMySanity
        | request |
        request := 'cat'  beMutable.
        self assert: request  = 'cat'.
        request at: 1 put: $b.
        self assert: request  = 'bat'.


Now if one changes the first line to either:

        request := 'cat' copy beMutable .

or

        request := 'cat' beMutable copy.

the test will always pass.
----
Roger Whitney              Department of Computer Science
[hidden email]        San Diego State University
http://www.eli.sdsu.edu/   San Diego, CA 92182-7720
(619) 583-1978
(619) 594-3535 (office)
(619) 594-6746 (fax)

To preserve the intention of the test, maybe it should be rewritten as

testMySanity
        | request |
  request := 'cat'  beMutable.
  self assert: request  = 'cat'.
  request at: 1 put: $b.
  self assert: request  = 'bat'.

  request at: 1 put: $c.
        request beImmutable.

This would restore initial condition.

HTH,
Michel.

Bany, Michel wrote:

IMO this is a bad idea:

--you are altering compiled code, such alterations will not show up in
the source leading to unpredictable behavior.

--when your test fails for some reason the code will not be reset to
it's original semantics.


So i suggest that using
   request := 'cat' copy.
is cleaner and more predictable, exactly what I require of a test.



Cheers!

Reinout
-------

Ah, OK.
I was (wrongly) assuming that the intention of this test was to test
the #beMutable selector.

> > --you are altering compiled code, such alterations will not
> > show up in the source leading to unpredictable behavior.
> >
> > --when your test fails for some reason the code will not be
> > reset to it's original semantics.
>
> Ah, OK.
> I was (wrongly) assuming that the intention of this test was to test
> the #beMutable selector.

Even if that is so then this is still a pitfall, you should not be mutating
literals since they are part of the compiled code. As said elsewhere making a
copy of the literal gives you an object that you may safely alter. Initially
the copy will be mutable but it is perfectly safe to make it immutable (for
instance for the purpose of a test) because you own that object.

In general you should not change the mutability of an object if you don't own
it. Doing so would break the contracts other parts of the system have
concerning that object (which may or may not be disastrous ;-).


HTH,

Reinout
-------

Two things:

        1.  IMO this entire situation should never be allowed to occur.
The issue of immutability of constant strings is a distraction; the
constant should never be accessed directly in the first place.
Assignment of a literal string to a variable should create a copy of the
literal and assign the *copy* to the literal.  In other words

                s := 'some string'

should produce the same bytecodes as

                s := 'some string' copy

Doubtless this would break some code somewhere which counts on the
current behavior (for example, code that uses #== to compare literal
strings).  Burn the disc packs...

        2.  Saying that one should not be mutating literals assumes that
one knows that a string is a literal.  For example

        someMethod: value
                self doSomethingTo: (self valueString: value)

        valueString: value
                value isNil
                        ifTrue: [ ^'nobody home' ]
                        ifFalse: [ ^value printString ]

        doSomethingTo: aString
                aString beMutable etc etc etc

Code in #doSomethingTo: doesn't know whether aString is or is not a
literal.

IMO immutable strings are fundamentally a hack and should be engineered
out of the image.  YMMV.

Bob

-----Original Message-----
From: Reinout Heeck [mailto:[hidden email]]
Sent: Thursday, February 02, 2006 2:00 PM
To: [hidden email]
Subject: Re: String beMutable SUnit bug/feature


> > --you are altering compiled code, such alterations will not
> > show up in the source leading to unpredictable behavior.
> >
> > --when your test fails for some reason the code will not be
> > reset to it's original semantics.
>
> Ah, OK.
> I was (wrongly) assuming that the intention of this test was to test
> the #beMutable selector.

Even if that is so then this is still a pitfall, you should not be
mutating
literals since they are part of the compiled code. As said elsewhere
making a
copy of the literal gives you an object that you may safely alter.
Initially
the copy will be mutable but it is perfectly safe to make it immutable
(for
instance for the purpose of a test) because you own that object.

In general you should not change the mutability of an object if you
don't own
it. Doing so would break the contracts other parts of the system have
concerning that object (which may or may not be disastrous ;-).


HTH,

Reinout
-------


-----------------------------------------
This message and any attachments are intended for the individual or
entity named above. If you are not the intended recipient, please do
not forward, copy, print, use or disclose this communication to others;
also please notify the sender by replying to this message, and then
delete it from your system. The Timken Company / The Timken Corporation

Jarvis, Robert P. (Bob) (Contingent) wrote:
Brrr, more complexity. Also it is an incomplete model, what if we don't
do assignment but still use a literal?

Copy all literals? regardless of whether assignment is done? That sounds
like a performance horror to me (specifically in parsers and such).

> Doubtless this would break some code somewhere which counts on the
> current behavior (for example, code that uses #== to compare literal
> strings).

What you don't seem to realize is that your proposal breaks OO itself.
Identity is central to OO but you propose to make literals 'magic' in
that literals cannot be #== to themselves anymore.


> Burn the disc packs...

Happily, if you pay us :-)
  (there are only 120000 methods in our image)



> doSomethingTo: aString
> aString beMutable etc etc etc

This violates my 'rule' of not making objects mutable if you don't own
them.

#doSomethingTo: has a contract with it's argument, it may alter the
argument and when the programmer sticks in an immutable object that will
show soon enough.
My point is that the *programmer* violated the contract and since
immutability was introduced the system nicely tells us.


> IMO immutable strings are fundamentally a hack and should be engineered
> out of the image.  YMMV.

It should be clear by now that I think 'automagically copying literals'
are a hack and that immutability is a tool ;-)




Cheers,

Reinout
-------

This all started because I was on a plane and was trying to write  
tests for a small server I was writing for class and needed the next  
day. I wanted a way to replace a ReadAppendStream but did not have  
access to the Mock object classes etc. The actual test contained:

        request := 'serverCommandSyntaxHere' beMutable  readWriteStream.

The beMutable was needed to allow request to be writable. The test I  
posted was an attempt to find the smallest test the would reproduce  
the odd behavior I was getting.

On Feb 2, 2006, at 5:48 AM, Bany, Michel wrote:
>
> Ah, OK.
> I was (wrongly) assuming that the intention of this test was to test
> the #beMutable selector.
>
>
>


----
Roger Whitney              Department of Computer Science
[hidden email]        San Diego State University
http://www.eli.sdsu.edu/   San Diego, CA 92182-7720
(619) 583-1978
(619) 594-3535 (office)
(619) 594-6746 (fax)

Roger Whitney wrote:

Right, which instead should be

request := 'serverCommandSyntaxHere' copy readWriteStream.

Cheers!

-Boris